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

Allow building ESMPy with setuptools/pip #49

Closed
wants to merge 7 commits into from

Conversation

milliams
Copy link

@milliams milliams commented Jun 13, 2022

As of Python 3.10, upon running python setup.py install there is an error printed

DeprecationWarning: The distutils package is deprecated and slated for removal in Python 3.12. Use setuptools or check PEP 632 for potential alternatives

The usual way to install packages in Python is using pip which follows the recommendations of PEP 632 and can use setuptools. See https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html for a write-up on this. However, if I try to install ESMPy with:

$ export ESMFMKFILE=...
$ cd src/addon/ESMPy
$ pip install .

then (after having to manually install numpy first) you get the error:

...
        File "/usr/lib64/python3.10/distutils/cmd.py", line 290, in set_undefined_options
          setattr(self, dst_option, getattr(src_cmd_obj, src_option))
        File "/usr/lib64/python3.10/distutils/cmd.py", line 103, in __getattr__
          raise AttributeError(attr)
      AttributeError: build_scripts
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
error: legacy-install-failure

× Encountered error while trying to install package.
╰─> ESMPy

It seems there's some subtle change with how distutils and setuptools work which causes this to fail.

This PR

This PR tries to make the minimal change to ESMPy to allow it to be installed with pip without changing how it behaves. It makes the following changes:

  1. Uses the setuptools.setup function explicitly to ensure modern behaviour
  2. The BuildCommand class now also derives from the pre-existing build command to get access to the required state.
  3. The esmfmkfile.py file is created in the build tree, not the source tree
  4. It no longer tries to import the library at build time, as there's no guarantee that any dependencies will have been installed at that time.
  5. numpy is specified as a dependency in setup.py so that it's automatically installed.

Future possible changes

Other potential changes are (you have no issue tracker I can find to suggest/discuss these):

  1. In emsfmkfile.py it is currently hard-coded at build-time to the value of ESMFMKFILE. I would change this to be able to read this env var at run-time with os.getenv("ESMFMKFILE", "hard/coded/path/from/build"). And if the ESMFMKFILE variable is not set at build-time, then it will just be os.getenv("ESMFMKFILE"). This will allow portable builds of ESMPy which can be pointed wherever ESMF is installed.
  2. Create a pyproject.toml which just has the minimum from https://setuptools.pypa.io/en/latest/userguide/quickstart.html#basic-use. This explicitly flags that you are conforming to the spec.
  3. setup.py currently does a lot and is being used like a Makefile. This is becoming less common and I believe it is deprecated. The test suite should be run directly with some runner (I think you use nose)
  4. Build a wheel of ESMPy since there's nothing but plain Python code in here. It can be easily installed and could then be distributed via PyPI like all other Python packages instead of just conda.
  5. Remove the option of build-time setting of ESMFMKFILE and require that it's set at runtime (the docs imply that this is possible, but it doesn't work). This would allow moving to a static build configuration like specified in PEP 621 (setuptools/flit/poetry are all options)

@rokuingh
Copy link
Contributor

@milliams Thanks so much for this contribution! We noticed the Python 3.10 issues while I was away for the last few weeks, so this is very well received. I just tried to pull your repo but it seems you have removed the non-ESMPy code, so I wasn't able to run the full test suite, which is required for us to merge a pr. If you have a full version let me know. Otherwise I'll try your changes on a full esmf version locally, and then merge this pr so you get the github fame and glory you are due :)

@milliams
Copy link
Author

Great :) I don't think I deleted any of the non-Python code, I can see it all at https://github.com/milliams/esmf/tree/fix_py_build. It looks like CircleCI is failing on a lot of the PRs so maybe there's some misconfiguration there?

@milliams milliams changed the title Allow building ESMPy with setuptoos/pip Allow building ESMPy with setuptools/pip Jun 15, 2022
@milliams
Copy link
Author

I've just updated this PR to also change the install docs to use pip.

The CI is still failing but that seems to be a persistent CircleCI bug.

@rokuingh
Copy link
Contributor

This pr was superseded by a more comprehensive pip update in PR #58. Many thanks to @milliams for spurring development on this important feature, and apologies for the duplicate PR that resulted in this one being closed without merge.

@rokuingh rokuingh closed this Sep 28, 2022
@milliams milliams deleted the fix_py_build branch November 17, 2022 13:31
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.

2 participants