search

mdshw5 / pyfaidx

Efficient pythonic random access to fasta subsequences
https://pypi.python.org/pypi/pyfaidx
Other
459 stars 75 forks source link

Migration to pyproject.toml? #186

Closed palao closed 1 year ago

palao commented 2 years ago

Hi, again. The current best practices in the python community is to use a static source of metadata for a project in a pyproject.toml file instead of using setup.py (even in combination with setup.cfg), see PEP 621. Is this something to be considered for this project? If yes, I offer a hand: I can make a PR...

PS notice that it has no impact on the end users that pip install the project. It's mostly relevant for developers. But I think it is a good thing to have.

mdshw5 commented 2 years ago

This looks like a good idea (thanks for sharing the PEP). I’d love some help on this as I’ve not used this system before.

palao commented 2 years ago

Great. I'll propose a PR asap.

palao commented 2 years ago

Question: how do you tag a release? Do you use bum2version, or something similar? Or do you just add a tag to the repo?

From what I see, I'd say you manually add a tag to the repo and let setuptools find the version. Is that true? If so, Would you mind to change the tagging procedure? It would make things more declarative..., more straightforward, and easier for the migration, I'd say

mdshw5 commented 2 years ago

Thanks for sharing this tool. I do currently tag releases, and use semantic versioning to build version numbers for test packages. The CI releases package builds to test-pypi and pypi using these versions, and stable builds are only pushed on new tags. I do like the current system, but could be convinced to change. However, I don't quite see why a different system is necessary, as development happens under VCS, and released versions contain explicit semantic version strings already.

palao commented 2 years ago

Preparing the PR for the transition to pyproject.toml I found something that has implications for the mechanics of setting the version number of the project.

I try to explain it here and also what I would do. It's just my observations and opinion, I don't pretend to impose anything. Also it is not a big thing after all...

From PEP 621, it is clearly advisable to use a static version number in the project.

Right now the version in pyfaidx is obtained dynamically using setuptools (pkg_resources.get_distribution). As far as I can see, that is the only thing that makes setuptools a runtime dependency: informing the user about the installed version of pyfaidx.

That has at least two negative side effects:

  1. It is required an additional dependency to just inform about the version number
  2. The version number is not static (i.e. to get the version we must run some code).

For point 1, of course I think we agree in the importance of distinguishing between development and runtime dependencies. One way to remove setuptools as a runtime dependency (not necessarily as development dependency) is to obtain the version for instance using importlib. This would fix point 1. However that solution would still imply a dynamic version number.

In my opinion point 2 is also a problem to be addressed: it would be very good if the __version__ attribute of the package was really the version number, wouldn't it? In that case the code itself is aware of the version number. And it is not necessary to call code to get it. And everything is more transparent, isn't it? That would also comply with PEP 621.

In short, my suggestion would be:

  1. Remove the setuptools way of computing the version number and simply replace that line by __version__ = "x.y.z" in pyfaidx.
  2. Use bump2version to, simultaneously, modify that version number and tag the repo (although the tag can be done manually or by other tools)
  3. Use Flit to create distributions and configure it such that the version number is directly read from the package __version__ attribute (this would mean replacing setuptools by Flit in the list of development dependencies).

I'll make a PR to show you something concrete, and you let me know... unless you think this is a no-go option.

mdshw5 commented 2 years ago

Hey @palao. I still need to take some time to digest this so thanks for your patience!

marcelm commented 2 years ago

From PEP 621, it is clearly advisable to use a static version number in the project.

It is totally fine to use dynamic version numbers, you just need to remember to write dynamic = ["version"] in the pyproject.toml. setuptools-scm, which pyfaidx uses, has a large user base and when PEP 621 was written, this use case was taken into account.

(You may have misread PEP 621: The section above the version number describes the package name, which indeed must be statically defined.)

To be honest, as much as I look forward to defining metadata in a standardized way in pyproject.toml, PEP 621 support in setuptools is still marked as experimental. I would wait one or two setuptools releases until migrating.