EMAworkbench icon indicating copy to clipboard operation
EMAworkbench copied to clipboard

Replace setup.py with pyproject.toml and implement optional dependencies

Open EwoutH opened this issue 1 year ago • 6 comments

This PR does two things:

  • It migrates the project install commands and metadata from setup.py to pyproject.toml. It largely follows the convention from setuptools.
  • It moves the dependencies from requirements.txt to the pyproject.toml file, and thereby also defines a few sets of optional depenencies. Note that many dependencies that were defined in the requirements.txt are now optional.

This PR succeeds #151 and closes #130.

To-do:

  • Migrate to pyproject.toml:
    • [x] Project and Metadata
    • [x] Dynamic version
    • [x] Dependencies and optional dependencies
    • [x] Version
    • [x] Packaging
  • [x] Update CI to use appropriate install commands for each job
  • [x] Update installation docs
  • [x] Implement "all" extras which installs everything

The installation.rst docs are also updated to reflect the new options for installing extras.

EwoutH avatar Sep 03 '22 12:09 EwoutH

Coverage Status

Coverage increased (+0.05%) to 80.051% when pulling 6ca01782f3dde2289841eec28b8a4f2929505806 on pyproject into da65c52401f502ae1bcf409e040e7d10d8c3ec6a on master.

coveralls avatar Sep 03 '22 12:09 coveralls

@quaquel One thing still needs to be implemented, and I need a bit of help from you on that. That is the packaing, or what files/directories to include in the package. Currently there is quite a large script for, but I'm sure we could simplify that.

Could you tell me which folders and files need to be exactly included in the package? Then I will take an initial jab at implementing that in the pyproject.toml.

EwoutH avatar Sep 03 '22 12:09 EwoutH

With a bit of reverse engineering I figured it out. :)

The reverse-wildcard trick is brand new and not even documented in Setuptools yet (I found it in a PR, and requested it to be documented in the Data Files Support docs).

EwoutH avatar Sep 03 '22 15:09 EwoutH

@quaquel Ready for review! Please also check the updated installation instructions, and if the right dependencies are assigned to each of the extras.

(please do not merge yet, even if everything looks goods, I want to clean the commits up before merging)

EwoutH avatar Sep 03 '22 18:09 EwoutH

You might have to walk me through all these changes in person. A couple of things

  1. In the past, I had bad experiences with the automated installation of dependencies. I have seen it brake quite a few anaconda installations. This might be from before pyproject.toml so it might no longer be a problem
  2. I am not sure whether we want to break down the dependencies to the degree you have done here. Yes, keeping a light installation can be useful in some cases, but it also creates a possible problem for users that might not fully understand all the configuration options that they have. Again, we can discuss the exact options in person.
  3. I would not call ["pysd"] vensim as is now done in the updated installation instructions. Vensim itself only requires the DSS version and is really a different connector from pysd. I suggest just calling ["pysd"] pysd.

quaquel avatar Sep 05 '22 18:09 quaquel

Thanks for reviewing! I will look into your points, let's walk through them Monday.

Can we do 16:00 by the way? I won't be able to make 15:30.

EwoutH avatar Sep 06 '22 09:09 EwoutH

@quaquel Turns out both JPype and pythonnet crash on other OSes than Windows.

In the latest commit:

  • Added graph to recommended extra
  • Removed the connector dependencies from all extra
  • Updated installation.rst docs

On my side it's ready, if you agree, I will clean it up and merge!

EwoutH avatar Sep 12 '22 15:09 EwoutH

@quaquel If you approve I can cleanup and merge, and modify #170 to work with the new setup.

EwoutH avatar Sep 19 '22 11:09 EwoutH

looks good so go ahead

quaquel avatar Sep 19 '22 12:09 quaquel