Android-FileBrowser-FilePicker icon indicating copy to clipboard operation
Android-FileBrowser-FilePicker copied to clipboard

replaced setup.py and -.cfg with pyproject.toml

Open venthur opened this issue 3 years ago • 4 comments

Since setuptools>=64 is is possible to fully replace setup.py and .cfg with pyproject.toml. It even works with editable installs, i.e. pip install -e .. This PR removes the former two files and replaces them with pyproject.toml. The translation is straightforward, except a few things:

  • console_scripts is now project.scripts and markdown.extensions is project.entry-points."markdown.extensions" see here
  • getting the version in setup.py was a bit convoluted, it is now a lot simpler using tool.setuptools.dynamic which gets the value dynamically from markdowon.__meta__.__version__
  • the development status is now hard-coded, I believe you won't regress from production/stable?
  • since setup.py does not exist anymore we now build using python -m build -- i cannot test the build-win target, since i don't have windows.

I compared the resulting, .whl and .tar.gz with the old ones, generated by setup.py, and they look identical to me.

venthur avatar Aug 30 '22 11:08 venthur

It is worth noting that support for [tool.setuptools] section is still in beta stage and might change in future releases:

https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html#setuptools-specific-configuration

I like it, but I would prefer to wait until it becomes stable.

mitya57 avatar Aug 30 '22 11:08 mitya57

Interesting. What is the value of doing this? Specifically, what we already had works just fine, so what do we gain by making this change? We are already familiar with the current system and are comfortable maintaining it. Why should we learn a new system when the old still works fine? To be clear, I am not opposed to adopting new stuff, but I just need to see a clear advantage to doing so. So, what is that advantage here?

  • getting the version in setup.py was a bit convoluted, it is now a lot simpler using tool.setuptools.dynamic which gets the value dynamically from markdowon.__meta__.__version__

The 'convoluted' method of getting the version was to avoid needing to import the entire markdown package (we only import __meta__ module). I don't recall why we needed to avoid that off-hand, but is that avoided by tool.setuptools.dynamic? If not, we may encounter that same issue. A search of our old bug reports should reveal the concern we were addressing with the current arrangement. And that would inform us of what to test for with the new approach.

  • the development status is now hard-coded, I believe you won't regress from production/stable?

I expect you are correct. Since I set that up, we have never used it for anything except production/stable.

I also share @mitya57's concern. That said, the only [tool.setuptools] keys we are using appear to be packages and version, both of which I expect are very unlikely to change. Regardless, we do want to approach with caution. This is why we need to see a clear advantage to adopting the change. Without a clear advantage, then we are taking on unnecessary risk.

waylan avatar Aug 30 '22 13:08 waylan

No need to merge immediately. Feel free to get comfortable with the toml file first. I assumed you want to go into that direction, as I found the pyproject.toml already but it didn't do much yet.

I assume in the long run, pyproject.toml will become the standard and replace setup.py. At least there have been a few PEPs in the last years that standardize on the build-system and the metadata in pyproject.toml.

Also, and probably a stronger indicator than my opinion: recently, the official packaging guidelines switched away from setup.py to pyproject.toml. This was the result of a lengthily discussion within the PyPA group when (not if) they can update the docs to recommend pyproject.toml instead of setup.py.

Anyways feel free to leave this open and update in the future if stuff changes in setuptools. I imagine if you want to move in that direction, this can be a good starting point.

The 'convoluted' method of getting the version was to avoid needing to import the entire markdown package (we only import meta module). I don't recall why we needed to avoid that off-hand, but is that avoided by tool.setuptools.dynamic? If not, we may encounter that same issue. A search of our old bug reports should reveal the concern we were addressing with the current arrangement. And that would inform us of what to test for with the new approach.

https://github.com/Python-Markdown/markdown/pull/1289/files#diff-50c86b7ed8ac2cf95bd48334961bf0530cdc77b5a56f852c5c61b89d735fd711R83-R84

These two lines do exactly that: they just import that file and read out the version. You can test it by just running python -m build -- if the package has the correct version, it worked. Also, if it wouldn't find the version, it would fail the whole build process.

To test if it produces usable packages in all supported python versions, your github actions should install the packages like so pip install -e . before testing. This will automatically build and install the package and fail if it fails to build.

venthur avatar Aug 30 '22 13:08 venthur

Thanks for addressing the concerns. That is helpful. Especially the following. I was not aware that that change had been made already.

Also, and probably a stronger indicator than my opinion: recently, the official packaging guidelines switched away from setup.py to pyproject.toml. This was the result of a lengthily discussion within the PyPA group when (not if) they can update the docs to recommend pyproject.toml instead of setup.py.

Also, I appreciate you doing this work. I was not looking forward to working through all of the various things in the setup.py script and figuring out how to reimplement them in pyproject.toml. You did that work us. If/when we make the change, we will use your work.

waylan avatar Aug 30 '22 14:08 waylan

This has been superceeded by #1324.

waylan avatar Apr 18 '23 14:04 waylan