has2k1 / plydata

A grammar for data manipulation in Python
https://plydata.readthedocs.io/en/stable/
BSD 3-Clause "New" or "Revised" License
275 stars 11 forks source link

Hot fix allow package to run #34

Open danich1 opened 1 year ago

danich1 commented 1 year ago

Closes #33

Greetings, I love this package and would like to see this work for Python 3.11. Due to a recent change, one gets this error:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/greenelabnlp/Documents/GitHub/plydata/plydata/__init__.py", line 1, in <module>
    from .one_table_verbs import *  # noqa
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/greenelabnlp/Documents/GitHub/plydata/plydata/one_table_verbs.py", line 6, in <module>
    from .operators import DataOperator
  File "/Users/greenelabnlp/Documents/GitHub/plydata/plydata/operators.py", line 6, in <module>
    from .eval import EvalEnvironment
  File "/Users/greenelabnlp/Documents/GitHub/plydata/plydata/eval.py", line 22, in <module>
    _ALL_FUTURE_FLAGS = _all_future_flags()
                        ^^^^^^^^^^^^^^^^^^^
  File "/Users/greenelabnlp/Documents/GitHub/plydata/plydata/eval.py", line 17, in _all_future_flags
    if feature.getMandatoryRelease() > sys.version_info:
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: '>' not supported between instances of 'NoneType' and 'sys.version_info'

Upon reading this error. I guess that feature.getMandatoryRelease() returns a None value, so I made a quick fix to check for that. Have confirmed the package can be loaded after my fix on Python 3.11.

zahlman commented 9 months ago

In detail: originally, the Postponed evaluation of annotations feature (__future__.annotations) was planned for Python 3.10, and this planning was done in time for 3.7. However, by the time Python 3.9 was released, the Python core development team determined that there were serious problems with the design of the feature, and postponed it initially to 3.11, and then again indefinitely - i.e., the feature is still not present in 3.12 and there is no announcement of when it, or any equivalent, will be added. There is another PEP for another attempt at the idea, but no clear consensus about how to proceed.

Since there is not any particular version of Python that is known to be "the version that must implement this feature", .getMandatoryRelease() indeed returns None instead of a version info tuple.

The documentation for the __future__ module is quite sketchy, and doesn't describe the possibility of this method returning None; but it seems like the only thing that makes sense given the current situation. Reading more closely, it is explicitly intended that code needs to check for a None value:

where, normally, OptionalRelease is less than MandatoryRelease, and both are 5-tuples of the same form as sys.version_info: ... MandatoryRelease may also be None, meaning that a planned feature got dropped.

(bold emphasis mine; other emphasis from the documentation)

Although I do not have them available to test, the issue presumably affects all current versions of Python 3.9 and up. Considering that 3.7 is currently past end-of-life, this is a pretty severe limitation; you may want to consider yanking some releases etc. @has2k1 At least please make sure to test on 3.9 and 3.10.

Re the proposed fix: it seems that the flags are intended to represent features that aren't yet mandatory; therefore, the code should update the value when the mandatory release version is None.