Skip to content

Handle dropped future features - #187

Merged
matthewwardrop merged 1 commit into
pydata:masterfrom
musicinmybrain:python3.11
Aug 16, 2022
Merged

matthewwardrop merged 1 commit into
pydata:masterfrom
musicinmybrain:python3.11

Conversation

@musicinmybrain

Copy link
Copy Markdown
Contributor

Fixes #186 by handling future features where the mandatory release version is None, indicating the feature was dropped. These are now included in patsy.eval._ALL_FUTURE_FLAGS rather than causing a TypeError.

The first appearance of such a feature is in Python 3.11, in which __future__.annotations.getMandatoryRelease() returns None.

Fixes pydata#186 by handling future features where the mandatory release
version is None, indicating the feature was dropped. These are now
included in patsy.eval._ALL_FUTURE_FLAGS rather than causing a
TypeError.

The first appearance of such a feature is in Python 3.11, in which
__future__.annotations.getMandatoryRelease() returns None.
Comment thread patsy/eval.py
# None means a planned feature was dropped, or at least postponed
# without a final decision; see, for example,
# https://docs.python.org/3.11/library/__future__.html#id2.
if mr is None or mr > sys.version_info:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If is dropped, should the next line run?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Answering that requires some reflection on what this code is trying to accomplish.

In this case, the PEP corresponding to the annotations feature was dropped in the sense that it was postponed twice, and a decision has not been made about whether it will be scheduled for a future release or rejected altogether. However, the feature is still available in __future__, and it might be scheduled for stabilization in the future.

I figured that this code is looking for future features that could have been imported and are not included in the current Python release. It seems like features with mandatory release None still satisfy this condition. It’s possible that I’m not seeing the full context, though.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the patch maked patsy work on my python-3.11.0rc1 test

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can see it looks like this code is trying to capture all possible flags that could be set by the user; so I think keeping the feature when mr is None makes sense.

@matthewwardrop matthewwardrop left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch @musicinmybrain! I'll merge this in and extend coverage to the Python 3.11 release, fixing any other issues that come up; and then put out a new patch release.

Comment thread patsy/eval.py
# None means a planned feature was dropped, or at least postponed
# without a final decision; see, for example,
# https://docs.python.org/3.11/library/__future__.html#id2.
if mr is None or mr > sys.version_info:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can see it looks like this code is trying to capture all possible flags that could be set by the user; so I think keeping the feature when mr is None makes sense.

@matthewwardrop
matthewwardrop merged commit bd3c12a into pydata:master Aug 16, 2022
@musicinmybrain

Copy link
Copy Markdown
Contributor Author

Thanks!

@EwoutH

EwoutH commented Oct 9, 2022

Copy link
Copy Markdown

Thanks for fixing this!

Would it be possible to get a new (patch) release with this fix included? Currently over at statsmodels our tests are breaking for Python 3.11 due to this issue.

@matthewwardrop

matthewwardrop commented Oct 9, 2022 •

Copy link
Copy Markdown
Collaborator

Hi @EwoutH , Thanks for the nudge! With scipy having started supporting 3.11, we could now push out support also. I've pushed out the 0.5.3 release.

Note: It would be nice to migrate statsmodels over to Formulaic at some point soon :). Generally it is more robust, has more features, and is much faster.

@EwoutH

EwoutH commented Oct 9, 2022

Copy link
Copy Markdown

Hi @EwoutH , Thanks for the nudge! With scipy having started supporting 3.11, we could now push out support also. I've pushed out the 0.5.3 release.

Thanks a lot for the quick response and the new release! I'm happy to report it unbreaks our Python 3.11 CI for Ubuntu and macOS! (Windows still has a separate issue)

Note: It would be nice to migrate statsmodels over to Formulaic at some point soon :). Generally it is more robust, has more features, and is much faster.

It sounds great! I'm not a core maintainer of statsmodels however, should we open an issue there to discuss it with them?

@matthewwardrop

Copy link
Copy Markdown
Collaborator

It sounds great! I'm not a core maintainer of statsmodels however, should we open an issue there to discuss it with them?

Hmm... I guess I should publish v1.0.0 of formulaic first (which will come with more complete documentation). In the mean time, patsy will continue working :). Thanks @EwoutH !

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Importing patsy.eval fails on Python 3.11

5 participants