Drop Python 3.8, 3.9 and add Python 3.12 #307
Conversation
In principle older versions will likely work, but as no wheel package is available on PyPI for Python 3.10 it is a nuisance to test. If this is really a problem for somebody I suppose we could build it in the CI.
3.10 as a float gets converted to 3.1 and causes all sorts of trouble
Now that the doc test has moved to newer Python/Numpy versions we have a doctest failure where numpy string types are unexpectedly encountered.
There is plenty more to do, but we do a bit here to check that no tests are still sneakily running on an older version of Python.
|
I have included a little bit of type-hinting cleanup to take advantage of Python 3.10 features. More of this can be done in a separate PR, but doing some here will help convince me that we really aren't still running this on Python 3.8 somewhere in the test suite. (We also drop the |
euphonic/readers/castep.py
Outdated
There was a problem hiding this comment.
Should these changes be part of this PR?
There was a problem hiding this comment.
It fixes a failure in the doc-tests, which only emerged now that the doc-tests are running on a newer Python/Numpy.
This could be discussed in a separate issue but as it is a one-liner with seemingly no negative consequences it might as well be rolled in here! It might merit a CHANGELOG mention... actually I forgot to update the changelog anway 🤦
| @@ -139,16 +139,15 @@ def run_setup(): | |||
| include_package_data=True, | |||
There was a problem hiding this comment.
In setup.py you could strictly do a check against python version and set requirements based on that and pass through. Still not sure that's wise, but worth noting.
There was a problem hiding this comment.
I don't think it's necessary here. It's a useful tool when things go wrong, but for now let's assume that the packages we are using all define their requirements correctly and make the installer responsible for getting a compatible set.
If we hit trouble with a dependency's metadata, that's the time to introduce special version-dependent pins.
| @@ -143,7 +142,7 @@ def run_tests(pytest_options: List[str], do_report_coverage: bool, | |||
| pytest_options = ['--import-mode=append'] + pytest_options | |||
|
|
|||
| # Start recording coverage if requested | |||
| cov: Union[coverage.Coverage, None] = None | |||
| cov: coverage.Coverage | None = None | |||
There was a problem hiding this comment.
Is it still recommended this be expressed as Optional? Fine either way.
There was a problem hiding this comment.
I prefer to only use Optional to indicate an optional parameter. Here we have a variable that, depending on code branches, may or may not be None; it's a different situation, where it is not so obvious what "Optional" would mean.
- Update docs Numpy requirement - Drop OrderedDict (standard dict works fine now, and the ordering is not _so_ important here that it is worth drawing extra attention.)
|
This won't hit conda-forge until the next release anyway, hopefully the fire will be out by then. Thanks for looking 😄 |
Python 3.9 isn't quite dead yet but supporting it on ARM Mac is quite annoying. 3.12 is very much out there in production now.