hnn-core
hnn-core copied to clipboard
[WIP] Build pyproj toml
Making a PR for @gtdang 's work transitioning to pyproject.toml
We should also use this as an opportunity to consolidate other top-level configuration files into pyproject.toml:
- [ ]
pytest.ini: seems we can https://docs.pytest.org/en/7.1.x/reference/customize.html#pyproject-toml - [ ]
.coveragerc: seems we can https://coverage.readthedocs.io/en/latest/config.html#toml-syntax - [ ]
.mailmap??? - [ ]
MANIFEST.in??? - [ ]
setup.cfgseems maybe not https://flake8.pycqa.org/en/latest/user/configuration.html#configuration-locations , but if we change torufflinting (see #934) then we can delete this file
fyi. I thought I could get away with adding mpi4py to the parallel optional dependencies because you can specify os conditions in the pyproject.toml. Like:
[project.optional-dependencies]
opt = ['scikit-learn']
...
parallel = ['joblib', 'psutil', 'mpi4py ; platform_system != "Windows']
It actually worked preventing it from installing on Windows however... it broke the macOS unit test workflows because it was mixing and matching: getting mpi4py from pip and OpenMPI from conda. It couldn't find OpenMPI in the path for some reason.
good idea to consolidate configuration in pyproject
I suggest making the changes iteratively in small PRs. For now, if it works with pytest.ini and coveragerc I would be a happy man
Regarding mpi4py, unfortunately it needs to have its install handled separately from the official Python dependencies of our package. We should definitely not include it in our pyproject.toml, and I have not yet updated the official installation documentation with my install investigation into OpenMPI install.
The parallel dependency group is a little weird... joblib is the default if you don't have MPI. The only reason you wouldn't want it installed is if you have MPI set up already... And then psutil is only used with MPI. If mpi4py must be externally installed I guess we should just pair psutil with that installation.
It might be best to get rid of that dependency group because it's super confusing.