insarlab / PySolid

A Python wrapper for solid Earth tides
GNU General Public License v3.0
63 stars 9 forks source link

version 0.2.4: move scipy/matplotlib/skimage into sub-func #41

Closed yunjunz closed 1 year ago

yunjunz commented 1 year ago
jhkennedy commented 1 year ago

@yunjunz where do you see this recommendation?

yunjunz commented 1 year ago

@yunjunz where do you see this recommendation?

Hi @jhkennedy, I was reading from here (https://conda-forge.org/docs/maintainer/adding_pkgs.html#build-host-and-run). My understanding from it is: scipy/matplotlib-base/scikit-image are supposed to be in requirements/run, which they are; but not recommended in the requirements/host, which was used during the building.

This is because of the change in setup.py in #40, where I use import pysolid.version to grab the version number. After that, the conda-forge feedstock testing failed due to the lack of the above 3 packages in https://github.com/conda-forge/pysolid-feedstock/pull/10.

I am aware that moving them into the sub-function is not ideal, am happy to hear if you have more elegant solution.

jhkennedy commented 1 year ago

@yunjunz ah gotcha!

Generally, the recommendation is to not import your package in setup.py as your build and runtime requirements get mangled.

I agree it's desirable to provide the version number as a package variable and dynamically generate the version from the git history for dev versions, but you can do that without needing to import the package using tools like setuptools_scm. It would give you most of what you're doing in version.py for free. We're using it in the hyp3_sdk and a few other packages and have been really happy with it.

I could open a PR for that if you'd like.

yunjunz commented 1 year ago

That would be great @jhkennedy!

And you are also more than welcome to change setup.py to setup.cfg with pyproject.toml, if that makes your PR easier.