IndEcol / country_converter

The country converter (coco) - a Python package for converting country names between different classification schemes.
GNU General Public License v3.0
214 stars 71 forks source link

Add build-system requirements and isort configuration to pyproject.toml #131

Closed mwtoews closed 1 year ago

mwtoews commented 1 year ago

This implements PEP 517 and PEP 518 to describe the build-system requirements in pyproject.toml.

Other things in this PR:

konstantinstadler commented 1 year ago

This sounds interesting. Would that change the workflow for building/uploading to pypi, or building the conda-forge?

coveralls commented 1 year ago

Coverage Status

Coverage: 97.03%. Remained the same when pulling 2b04af13b091b7aaecc56043b98838ab2a3d6a8a on mwtoews:pyproject-toml into fdd1087c1b6ccfde22608f7234bf8e1304a9a0a5 on konstantinstadler:master.

konstantinstadler commented 1 year ago

Also, it requires the build package, so this probably should go into the environment.yml file?

mwtoews commented 1 year ago

This simplifies the workflow for build/upload for PyPI since you don't need to install the build-time dependencies (often setuptools and wheel), as this is done in this PR by build. Both sdist and wheels are built and uploaded to PyPI. And no, build should not be added to environment.yml, since it is not a run-time dependency.

I don't believe anything else needs to be done with https://github.com/conda-forge/country_converter-feedstock/blob/main/recipe/meta.yaml

konstantinstadler commented 1 year ago

Mmmh, but where does build gets installed in your pull request?

Do the changes in pyproject some magic? Generally, we have the environment.yml as a development environment (it also contains isort, pytest and such things).

mwtoews commented 1 year ago

build is installed in this PR here: https://github.com/konstantinstadler/country_converter/blob/2b04af13b091b7aaecc56043b98838ab2a3d6a8a/.github/workflows/publish_pypi.yml#L32 (along with twine to check the packages before they are uploaded).

But I still don't think build should be added to environment.yml, since it is not a runtime or development dependency (pytest, isort, etc.). Or at least I haven't seen it appear as a dependency with other projects. Typically, it's just pip install build followed by python -m build in a CI workflow used for building and publishing.

Also, I should note that when someone does pip install country_converter with the current versions of pip, it shows a warning messsage:

DEPRECATION: country_converter is being installed using the legacy 'setup.py install' method, because it does not have a 'pyproject.toml' and the 'wheel' package is not installed. pip 23.1 will enforce this behaviour change. A possible replacement is to enable the '--use-pep517' option. Discussion can be found at https://github.com/pypa/pip/issues/8559

This PR will obviously resolve the warning.

konstantinstadler commented 1 year ago

Fantastic - thanks for the follow up (sorry, overlooked the pip install in the workflow).