Conversation
Update the Python packaging method to use a modern pyproject.toml instead of the legacy setup.py only mechanism. Changed the PyLandau source back to the original package since it has been rewritten to remove the Cython dependency. Removed the env.yamls as they were for a deprecated method for installing with conda. Removed setup.py as it is no longer needed with the pyproject.toml Edited the README to remove references to the conda install method.
Switch back to the original PyLandau package.
mjkramer
left a comment
mjkramer
left a comment
There was a problem hiding this comment.
Tested in Pythons 3.11 and 3.13 (fresh install + minimal run on 2x2 MC file). Looks good to me for merging (but see comment on pyrpoject.toml regarding h5flow installation).
| @@ -14,4 +14,4 @@ dependencies: | |||
| - pip: | |||
| - pyyaml-include==1.3.2 | |||
| - adc64format>=0.0.2 | |||
There was a problem hiding this comment.
While we're here: If at some point we add tests that use newer light files, the old PyPI release of adc64format will cause problems. The github release listed in pyproject.toml will work, of course. Would be nice if we could avoid this duplication of dependency specifications...
Anyway, beyond the scope of this PR. Just thinking out loud.
There was a problem hiding this comment.
Actually I think we can just get rid of all the dependencies here, given that the test workflow does a pip install . and will pick everything up from the pyproject.toml.
There was a problem hiding this comment.
Given that we've given up on using MPI in practice, we can probably get rid of this file and replace it with env-nompi.yaml (renamed to env.yaml?)
| "scikit-image", | ||
| "scikit-learn>=1.3.0", | ||
| "dbscan1d==0.2.3", | ||
| "h5flow>=0.2.0", |
There was a problem hiding this comment.
This will fail (and always has failed) if the user doesn't already have h5flow installed in their environment. We might want to do folks a favor by changing this to e.g.
"h5flow @ git+https://github.com/DUNE/h5flow.git@v0.2.5#egg=h5flow",
(Maybe we leave the tag unspecified?)
There was a problem hiding this comment.
That's a good point. I changed the h5flow dependency to the git repository as you mentioned, but left out the explicit tag.
Change h5flow dependency to be the git repository so that it will install if not found. Updated README to reflect that manually installing h5flow is no longer necessary -- moved to a "legacy install" section.
2 participants
Summary of changes:
env.yamlfiles as they were for a deprecated method for installing with conda.setup.pyas it is no longer needed with thepyproject.tomlTested with a single 2x2 MR6.5 file that ran without error. Could use more careful validation that the file is the identical, or close enough.
Creating the PR to have a more visible reminder to merge this in at some point, but it's not that high priority.