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

Fixes for Python 3.12 #174

Merged
merged 8 commits into from
Oct 26, 2023
Merged

Fixes for Python 3.12 #174

merged 8 commits into from
Oct 26, 2023

Conversation

bennybp
Copy link
Contributor

@bennybp bennybp commented Oct 16, 2023

This fixes versioneer to be compatible with python 3.12. This also adds python 3.11 and 3.12 to the Github Actions tests.

The fix to versioneer is somewhat hacky (rather than vendor a new versioneer.py from the not-yet-released versioneer package).

This also replaces the use of distutils in nifty. distutils was removed in python 3.12 (but is still available from setuptools). The copytree command from distutils (with dirs_exist_ok=True) should be equivalent for python 3.8+. A test was added for the copy_tree_over function.

Fixes issue #173

@leeping
Copy link
Owner

leeping commented Oct 20, 2023

Thank you, Ben. I don't see any issue with merging this into master. :)

@leeping
Copy link
Owner

leeping commented Oct 25, 2023

Thanks a lot for the recent fixes. Is it possible to use importlib.metadata instead of packaging? Because geomeTRIC has only two critical dependencies (only numpy and networkx), I would prefer not to introduce a dependency on a third-party package in order to keep an existing functionality. I believe importlib.metadata also allows one to access version numbers of installed packages: https://docs.python.org/3/library/importlib.metadata.html .

We would need to drop Python 3.7 in the next release if we go this route and I'm okay with that.

@bennybp
Copy link
Contributor Author

bennybp commented Oct 25, 2023

Actually, it's used in one place, to check the version of networkX.

The check is to see if networkX is > 2.0, which was released in 2017. It is probably safe to remove that check, since that is a pretty old version now. It's likely if you try to install 2.0 other things will break before geometric does.

There's one more thing that likely needs a fix: In the test-conda.yml github action, setup-miniconda should probably be used rather than adding $CONDA to the path and installing mamba manually. We've found it to be much more robust (as seen with this failure)

@leeping
Copy link
Owner

leeping commented Oct 26, 2023

If you merge the latest changes from master, the Conda environment should now be set up correctly.

geometric/molecule.py Outdated Show resolved Hide resolved
@leeping
Copy link
Owner

leeping commented Oct 26, 2023

The tests indicate the optimization log file is not being generated for an OpenMM test. (The other OpenMM tests do not involve parsing the log file). Did the logging behavior change somehow?

@bennybp
Copy link
Contributor Author

bennybp commented Oct 26, 2023

Wow sorry, that's me being sloppy. My latest commit hopefully fixes all those

Copy link
Owner

@leeping leeping left a comment

Choose a reason for hiding this comment

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

The latest changes all look good!

@bennybp
Copy link
Contributor Author

bennybp commented Oct 26, 2023

Great! You can probably squash and make this cleaner when you merge

@leeping
Copy link
Owner

leeping commented Oct 26, 2023

LGTM!

Sounds good regarding the squash, I'll do that.

@leeping leeping merged commit 1766bc6 into leeping:master Oct 26, 2023
10 checks passed
@effigies
Copy link

The versioneer issue has been resolved for some time, but an old versioneer is vendored here. Upgrade with pipx run versioneer install.

See https://github.com/python-versioneer/python-versioneer/blob/master/UPGRADING.md for any migration issues.

leeping pushed a commit that referenced this pull request Jan 24, 2024
* Hacky fixes to versioneer for python 3.12

* Add python 3.11 and 3.12 to GHA tests

* Deprecate distutils and add test

* Change python version detectino to < 3.8

* rdkit is not 3.12 compatible yet

* Remove dependence on pkg_resources

* Remove need for packaging/parse_version

* Some test fixes
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.

None yet

4 participants