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

Switch to declarative package management #257

Merged
merged 25 commits into from
May 6, 2022

Conversation

gipert
Copy link
Member

@gipert gipert commented Apr 27, 2022

I've been mainly following recommendations from here: https://scikit-hep.org/developer/packaging

  • packaging instructions are now contained in pyproject.toml, setup.cfg (and few lines in setup.py to be backward-compatible)
  • package source has been moved below src/ as recommended: simplifies packaging by a lot
  • the pygama version is now managed by setuptools, which computes a git-describe-like version string automatically. I didn't know about this before and I consider it robust enough (requires just a couple of extra configs in pyproject.toml and no extra custom logic). We will not need to manually advance the version string anymore.
  • export only pygama version string as LH5 object

Still to do:

@jasondet: are you OK with moving the source below src/? Do you have any particular development requirement/need?

To be merged after #255

Mainly from here: https://scikit-hep.org/developer/packaging

* packaging instructions are now contained in pyproject.toml, setup.cfg
  and setup.py
* move package source below src/, ase recommended
* deleted unused pytest.ini
* new version management system with setuptools, computes version string
  automatically
* export only pygama version as LH5 object
@gipert gipert added the packaging Project packaging label Apr 27, 2022
@gipert gipert self-assigned this Apr 27, 2022
@gipert gipert changed the title Switch to better package management practices Switch to declarative package management Apr 27, 2022
@gipert gipert marked this pull request as ready for review April 29, 2022 10:48
@gipert
Copy link
Member Author

gipert commented Apr 29, 2022

I'm done with this PR – let me know what you think @jasondet @iguinn and feel free to merge, if no objections. legend-exp/pyfcutils#2 should be merged too to make fcutils install seamlessly.

@jasondet jasondet requested review from wisecg and iguinn May 2, 2022 15:26
@jasondet
Copy link
Collaborator

jasondet commented May 2, 2022

oh wow. this looks like a good idea to me. I've requested that @iguinn and @wisecg review it.

@wisecg
Copy link
Collaborator

wisecg commented May 2, 2022

Wow! I'm impressed at the work here. I support any change that makes pygama more aligned with best practices in ScikitHEP. Eventually it would be great to publish this package with them.

The only thing I note here is that we would move the "pygama" dir one level down, to "src/pygama". As long as Luigi feels that's necessary, I'll support his decision.

@gipert
Copy link
Member Author

gipert commented May 3, 2022

Wow! I'm impressed at the work here. I support any change that makes pygama more aligned with best practices in ScikitHEP. Eventually it would be great to publish this package with them.

I guess it would be nice to contact them at some point, yes: https://scikit-hep.org/affiliated. But ultimately it will depend on pygama's ability to be not-so-much LEGEND-specific.

The only thing I note here is that we would move the "pygama" dir one level down, to "src/pygama". As long as Luigi feels that's necessary, I'll support his decision.

Yes, it's generally highly recommended as most tools assume this structure by default (and therefore no extra configuration is needed): https://scikit-hep.org/developer/pep621#package-structure

@jasondet
Copy link
Collaborator

jasondet commented May 6, 2022

ok. I'm going to merge this...

@jasondet jasondet merged commit a703677 into legend-exp:refactor May 6, 2022
@gipert gipert deleted the packaging branch May 7, 2022 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packaging Project packaging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants