Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace setup.py by pyproject.toml #33

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

coroa
Copy link

@coroa coroa commented Feb 9, 2023

The use of setup.py is being superseeded by declarations in pyproject.toml slowly.

https://packaging.python.org/en/latest/specifications/declaring-project-metadata/

Here i am migrating everything from setup.py there.

Explicit packages, package_dir and py_modules have been removed in favour of setuptools default src-layout-based "Auto-Discovery" (https://setuptools.pypa.io/en/latest/userguide/package_discovery.html#auto-discovery).

include_package_data was omitted since that is on by default.

@joaomcteixeira
Copy link
Owner

Dear @coroa

Many thanks for your contribution. I will review it asap in the next few days.

- corrected some typos
- added the client
- removed `setup.py` from `tox.ini`
@joaomcteixeira
Copy link
Owner

Hi @coroa

I corrected some typos in the pyproject.toml file and started editing the tox.ini. Many thanks for this contribution. I won't merge yet because I want to review the whole process and update documentation as well to make sure everything is harmonized. I will keep working on your branch. Feel very welcome to continue working on this PR as you wish. Otherwise, I will finish it as I have time in the next few days.

Best,

@joaomcteixeira joaomcteixeira added enhancement New feature or request good first issue Good for newcomers labels Feb 9, 2023
- removed usage of `setup.py`
- updated packing commands

**to do:**
- `test` env still won't work
- `docs` env still won't work
- test package README builds correctly
@joaomcteixeira
Copy link
Owner

Not straightforward to configure tox with pyproject.toml, it seems many integrations between both projects are still not fully developed. Meanwhile, we continue to prepare this PR.

@coroa
Copy link
Author

coroa commented Feb 16, 2023

True, i haven't used tox so much previously. So, did not foresee the integration issues. Thanks for taking it along anyway. (And sorry for the typos)

@joaomcteixeira
Copy link
Owner

Thanks @coroa

I will keep working on it for the next few weeks while seeing how tox goes along with it or if it's something I am doing wrong. Sorry for any delays; very busy at work recently.

I found it difficult for me to go along without tox because it's really an excellent tool for teamwork and reproducibility.

@coroa
Copy link
Author

coroa commented Feb 21, 2023

All tox envs work fine for me here, except that i did not try tox -e pr, but i'm on a fairly recent python mac os x install (3.10.10).

@coroa
Copy link
Author

coroa commented Feb 21, 2023

On a different note, i got rid of bumpversion and check-manifest in favour of setuptools_scm and replaced isort and flake8 with ruff and black over at https://github.com/coroa/python-project-skeleton/tree/use-automatic-versioning . Since this completely takes away your versioning workflow, i do not intend to PR the whole bunch, but if your interested in bits and pieces. Feel free

@codecov-commenter
Copy link

codecov-commenter commented Feb 21, 2023

Codecov Report

Merging #33 (535927f) into main (42e139b) will not change coverage.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##             main      #33   +/-   ##
=======================================
  Coverage   93.33%   93.33%           
=======================================
  Files           4        4           
  Lines          15       15           
  Branches        0        2    +2     
=======================================
  Hits           14       14           
  Misses          1        1           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@joaomcteixeira
Copy link
Owner

Thanks very much, @coroa, for this contribution. I made all tests pass. I want to review the documentation to ensure everything is updated.

On another more philosophical note, and to keep the discussion active 😉, I understand the Python community is moving towards the pyproject.toml and some benefits exist. But, for me, it is not clear the true benefit of using the toml file strategy. With setup.py, we could configure as much as needed and run custom scripts that were useful in some cases for the packing or installation. I may be missing the big picture, though.

Also, I disagree with placing all configurations in the pyproject.toml, as is the current tendency. For example, I prefer flake8 and isort parameters in tox.ini where they are used or in their own files. Simpler to configure and easier to find, IMHO.

I may be wrong, but I see a loss of (some) capabilities. But, in this case, I will adopt this strategy because that's where the community is moving and adopting the toml is a general "new practice" rather than an opinion.

Many thanks for putting this forward. Hope soon I can finish reviewing it and merge it.
Cheers,

@coroa
Copy link
Author

coroa commented Feb 21, 2023

Also, I disagree with placing all configurations in the pyproject.toml, as is the current tendency. For example, I prefer flake8 and isort parameters in tox.ini where they are used or in their own files. Simpler to configure and easier to find, IMHO.

I actually was not familiar with tox enough which predisposed me against tox.ini. I thought the config there would be exclusive to the invocation by tox. ie. the flake8 linter in vscode would be behaving differently. After checking the documentation for a couple of tools (flake8 and isort), I stand corrected.

I have a slight preference for less files, so i tend to embrace the pyproject.toml strategy; but i am also still fine with separate files for the different tools. I do not see a difference between tox.ini and pyproject.toml.

It currently feels like everything would converge toward the latter eventually.

@joaomcteixeira
Copy link
Owner

I actually was not familiar with tox enough which predisposed me against tox.ini. I thought the config there would be exclusive to the invocation by tox. ie. the flake8 linter in vscode would be behaving differently. After checking the documentation for a couple of tools (flake8 and isort), I stand corrected.

Very interesting. I was not aware tools like vscode were not reading from tox.ini. I am vim user, and I don't like to have any tools/plugins incorporated when editing code. In this particular case, when I want to check the lint for the current file, I do ,k which is a custom hotkey for !tox -e lint -- % inside vim. Thus, tox runs in the background. But your argument is very valid and is reason enough to move the configuration to pyproject.toml if other editors can read from it.

I have a slight preference for less files, so i tend to embrace the pyproject.toml strategy; but i am also still fine with separate files for the different tools. I do not see a difference between tox.ini and pyproject.toml.

IMHO, the difference is that pyproject.toml is about defining the package. And tox is about the CI activities. Hence, I see flake8, black, or ruff (etc) should be defined separately from the project. Plus there is more complexity in defining such parameters in the pyproject.toml with the tool. or .init_ pre/suffixes.

It currently feels like everything would converge toward the latter eventually.

That's true. 😉 Good conversation!

@tomschr
Copy link

tomschr commented Apr 14, 2023

@joaomcteixeira Found this by accident (seems I have the tendency to leave a comment in arbitrary issues). 😉 🙈

I saw that you've use bumpversion to modify the version in the pyproject.toml file. If I'm not mistaken, this is not really necessary. The setuptools library distinguish between static information (like authors, project name etc.) and dynamic information (like versions).

You could amend the TOML file with this:

[project]
# ...
dynamic = ["version"]

[tool.setuptools.dynamic]
version = {attr = "mymodule.__version__"}

--- Taken from pypa/setuptools#3190

I have to admit, it's currently in beta so you might get some warnings.

See also https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html#dynamic-metadata

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants