kiyo-masui / bitshuffle

Filter for improving compression of typed binary data.
Other
215 stars 76 forks source link

Build backend updated to poetry core #119

Open danielemichilli opened 2 years ago

danielemichilli commented 2 years ago

This solves issue #118 and makes the repository compatible with poetry. To do so, it updates the style of pyproject.toml and uses its light weight, fully compliant, self-contained package allowing PEP 517 compatible build frontends, solving the error indicated in the issue

jrs65 commented 2 years ago

Can you explain more why the issue you link to is an issue in bitshuffle? That poetry doesn't install the listed build requirements before trying to buld the package seems like an issue in poetry (quite possible this issue: https://github.com/python-poetry/poetry/issues/2789).

Generally I'm interested in moving a bunch of packages to more modern build methods but in my (limited) past experience poetry is the most frustrating of them all. At the moment the current setup.py/setuptools builds mostly works fine so a high premium should be placed on changing it over.

Are there tweaks to the pyproject.toml file that would make poetry install Cython without changing the build backend to poetry?

jrs65 commented 2 years ago

Anyway @danielemichilli I'm on vacation for two weeks, but it would be good to figure out the pros and cons of this when I'm back. Otherwise Kiyo might be able to deal with it in the mean time.

danielemichilli commented 2 years ago

This is not necessarily an issue with bitshuflle but I think making it poetry-compatible is a good idea if it does not cause problems with the module. I was able to install bitshuffle without issues after my edits with or without poetry but I am not aware of all the building methods that are necessary to be supported. I believe this would be a quick test to run, in the meantime, enjoy your vacation!

Are there tweaks to the pyproject.toml file that would make poetry install Cython without changing the build backend to poetry?

I have looked into this but I did not find a way to do it.

shinybrar commented 2 years ago

@danielemichilli @jrs65 -- looking into this issue and here are some observations.

You need Cython installed in the environment to even evaluate the install and setup directives. This breaks any setup command other than install.

Most third-party dependency resolution systems, pipx, poetry, flit etc. use egg_info to fetch metadata, build tags and dependency links for the project. Note, this is outside the purview and different from install_requires and setup_requires dependencies which are correctly defined in pyproject.toml.

The solution is quite simple,

  1. Move the cython imports under a separate function
  2. Call the said function from within setup() which is invoked after setup_requires.
shinybrar commented 2 years ago

I would recommend closing this PR in favor of #123