Migrate config files into pyproject.toml

[banesullivan]

There are a lot of files at the root level of the repository. I thought it'd be nice to embrace setup.cfg (and try to move away from pyproject.toml) to combine many of these files into a single configuration. Unfortunately, Black has a very harsh stance on not supporting setup.cfg (see psf/black#683 (comment) and previous comments.... I guess harsh opinions are what you get when you use an "uncompromising code formatted")

This paired with #3327 should clean up the root level of the repository a bit

Note: I wish we could drop pyproject.toml altogether as it leads to a lot of headaches and I think conflicts with setup.cfg (isn't pyproject.toml a Poetry-specifc thing? I don't really know anything about this...)

[codecov]

Codecov Report

Merging #3328 (49b029e) into main (b99e5f8) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #3328   +/-   ##
=======================================
  Coverage   95.17%   95.17%           
=======================================
  Files          83       83           
  Lines       18571    18571           
=======================================
  Hits        17675    17675           
  Misses        896      896           

[adeak]

Without claiming to know much about this, the brief history seems to be setuptools (third-party packaging library) coming up with setup.py, then switching over to setup.cfg due to code execution vulnerabilities inherent in setup.py. Then PEP 517 and PEP 518 defined pyproject.toml that is

1. independent from setuptools
2. has a well-defined specification.

So "pyproject.toml is a Poetry-specific thing" is only the case to the extent of "pyproject.toml is a non-setuptools thing", as I understand. See also the setuptools issue titled "Eventually deprecate setup.cfg with automatic conversion to pyproject.toml ", and the section in PEP 518 titled "Sticking with setup.cfg".

Things move slowly in the packaging world, but pyproject.toml seems to be the sustainable path forward in the long term.

[adeak]

@akaszynski should also weigh in here I think.

[akaszynski]

Things move slowly in the packaging world, but pyproject.toml seems to be the sustainable path forward in the long term.

I'd like to completely migrate to pyproject.toml rather than going backwards to setup.cfg. setup.py is only needed if you build projects with c-extensions, but I imagine that will eventually transition.

Personally, I used to prefer setup.py, but from what I've seen at https://github.com/pyansys, you can do some really dumb things with setup.py and pyproject.toml is better.

I think poetry is overkill for this project and flit permits pip install -e without getting sucked into the poetry ecosystem. It's the most like setup.py.

[banesullivan]

I'd like to completely migrate to pyproject.toml

I don't know if I'm on board with this... you and I may have to chat offline, so I understand all of this better. As of right now, pyproject.toml gives me headaches and issues in certain environments as many build tools force you into Poetry with it.

All of this is new territory for me, so I'm happy to embrace what others think is best here.

But I do want to move as many configs into a single file as possible. Should I change this PR to move everything into pyproject.toml?

[akaszynski]

But I do want to move as many configs into a single file as possible. Should I change this PR to move everything into pyproject.toml?

Preferably. I can show what we've done for pyansys. There's a team of ~7 people dedicated to figuring out python packaging and I think we've done our homework in this regard. I too agree that poetry is overkill. flit is the way to go to be able to migrate to pyproject.toml without being overwhelmed by poetry's hardheadedness.

chat offline

Agreed. Whatever conclusion we come to I'll post it here and the rest of the core team can weigh in.

[banesullivan]

Sounds good, I will update this PR to use pyproject.toml and we'll chat whenever

[banesullivan]

Switched to pyproject.toml. Codespell is having issues using the pyproject.toml file even though it is supposed to support it...

[banesullivan]

Merking as ready for review to get some fresh eyes on this so we can go ahead and wrap it up

[adeak]

Switched to pyproject.toml. Codespell is having issues using the pyproject.toml file even though it is supposed to support it...

According to its readme

Codespell will also check in the current directory for a pyproject.toml [...] file, and the [tool.codespell] entry will be used as long as the tomli package is installed

(emphasis mine). I haven't checked but that package doesn't ring a bell.

[banesullivan]

I haven't checked but that package doesn't ring a bell.

That package is installed locally for me but codespell doesn't register the pyproject.toml still

[MatthewFlamm]

This looks good to me in general. It looks like we have to wait for a new codespell version to be released to implement this, or semi-dangerously move to a git dev-branch based install. I think we need to add tomli to the pre-commit hook install too:

pyvista/.pre-commit-config.yaml

Lines 22 to 29 in bf82d18

	- repo: https://github.com/codespell-project/codespell
	rev: v2.2.1
	hooks:
	- id: codespell
	args: [
	"doc examples examples_flask pyvista tests",
	"*.py *.rst *.md",
	]

I'm not really sure how the pre-commit hooks decide on dependencies, but pip based install could use codespell[toml]. Or it could be added explicitly as an additional_dependency.

[banesullivan]

Codespell v2.2.2 was released and this should be good to go. Admittedly, I don't use pre-commit very often so someone should probably doubly check that everything is working as expected.

[MatthewFlamm]

I fixed codespell, but I'm not sure flake8 supports pyproject.toml. see https://flake8.pycqa.org/en/latest/user/configuration.html#configuration-locations and this package https://github.com/john-hen/Flake8-pyproject. I know nothing about this later package other than it claims to fix this issue.

[MatthewFlamm]

Doing a bit more digging, it looks like the maintainer(s) of flake8 is(are) decidedly against supporting configuration in pyproject.toml. See PyCQA/flake8#234. I think there are some good points in favor of not supporting it as the only option like black does, but I'm not sure that I follow the resistance to allowing it as one of several options other than maintainability. If PyVista ever allows configuration using one of these files, we will have to confront these questions as well.

The tools that wrap flake8 to support it seem flaky in that they require various patching techniques and/or merging upstream changes. I would unfortunately recommend removing flake8 entirely from this PR.

[adeak]

Doing a bit more digging, it looks like the maintainer(s) of flake8 is(are) decidedly against supporting configuration in pyproject.toml. See PyCQA/flake8#234.

One of the greater objections seemed to have been the lack of an stdlib toml parser. I wonder if they'll reconsider in light of https://docs.python.org/3.11/library/tomllib.html#module-tomllib new in 3.11.

Actually, the last comment addresses this PyCQA/flake8#234 (comment). Tl;dr waiting for pip to change behaviour (which is unlikely to happen).

[banesullivan]

I've moved the Flake8 config back to it's own .flake8 file in light of them not supporting pyproject.toml.

I belive this PR is ready to merge as of now...

[akaszynski]

This is good to go. I'd like for us to move fully to away from setup.py and to pyproject.toml.

Another move I'd like to make is the move of pyvista into the src directory. This would eliminate issues like testing the "local" directory instead of the installed library.

[banesullivan]

Another move I'd like to make is the move of pyvista into the src directory. This would eliminate issues like testing the "local" directory instead of the installed library.

Ooo... I don't know about this one... we'll have to chat or open an issue with a detailed justification