Handle dropped future features #187
Conversation
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.
| # 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: |
There was a problem hiding this comment.
If is dropped, should the next line run?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
the patch maked patsy work on my python-3.11.0rc1 test
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| # 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: |
There was a problem hiding this comment.
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.
|
Thanks! |
|
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. |
|
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 |
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)
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 ! |
Fixes #186 by handling future features where the mandatory release version is
None, indicating the feature was dropped. These are now included inpatsy.eval._ALL_FUTURE_FLAGSrather than causing aTypeError.The first appearance of such a feature is in Python 3.11, in which
__future__.annotations.getMandatoryRelease()returnsNone.