Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix undefined name sre_constants in Python 3.11 #552

Merged
merged 2 commits into from
Dec 5, 2022

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Dec 5, 2022

#546 (comment)

@gopackgo90 Your review, please.

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • My change requires a change to the dependencies
  • I have updated the dependencies accordingly
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@cclauss cclauss marked this pull request as ready for review December 5, 2022 07:40
@Pierre-Sassoulas
Copy link
Collaborator

There's still a warning in 3.11:

tests/execution/test_execution.py::test_non_subpath
  module 'sre_constants' is deprecated

@cclauss
Copy link
Contributor Author

cclauss commented Dec 5, 2022

Good catch. Thanks.

@Pierre-Sassoulas Pierre-Sassoulas merged commit 6812b5a into landscapeio:master Dec 5, 2022
@Pierre-Sassoulas
Copy link
Collaborator

👍

@cclauss cclauss deleted the patch-1 branch December 5, 2022 11:16
@gopackgo90
Copy link
Contributor

I'm a little surprised this works on Python < 3.11, because on my own system, doing this sort of import doesn't work for me on Python 3.8:

$ python
Python 3.8.13 (default, Aug 16 2022, 12:16:29) 
[GCC 9.3.1 20200408 (Red Hat 9.3.1-2)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import re._constants
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ModuleNotFoundError: No module named 're._constants'; 're' is not a package

I had anticipated a fix would be to do conditional imports like either jwilk/pydiatra@b69f3fc or The-Compiler/hypothesis@66ca462.

@cclauss
Copy link
Contributor Author

cclauss commented Dec 5, 2022

Interesting because the CPython change that you highlighted was from 2008 (Python 3.0.0) which proceeds by a decade

The changes made work like this...

>>> import re
>>> re._constants
<module 're._constants' from '/opt/homebrew/Cellar/python@3.11/3.11.0/Frameworks/Python.framework/Versions/3.11/lib/python3.11/re/_constants.py'>

@gopackgo90
Copy link
Contributor

Doing the import that way on Python 3.8 I get:

$ python
Python 3.8.13 (default, Aug 16 2022, 12:16:29) 
[GCC 9.3.1 20200408 (Red Hat 9.3.1-2)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import re
>>> re._constants
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: module 're' has no attribute '_constants'

@cclauss cclauss mentioned this pull request Dec 5, 2022
11 tasks
@Pierre-Sassoulas
Copy link
Collaborator

The tests are green, but maybe this file isn't even imported ?

@cclauss
Copy link
Contributor Author

cclauss commented Dec 5, 2022

Or the two exceptions are never triggered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants