Modernisation of repo

[jonas-eschle]

Hi @arogozhnikov , thanks for the repo, we still use it today! However, things are getting a bit outdated, i.e. the supported Python versions (theano is not available on newer releases), pyproject.toml etc.

Would you mind if I upgrade the repo? Remove parts that are technically no longer supported (or make them optionally dependend, upgrade packaging to modern standards, reformat with ruff and all of that?

[arogozhnikov]

Hey Jonas,

yes, I'm very open to this, help would be greatly appreciated!

Some thoughts:

• ruff check and format: certainly yes, my ask is please set line-length to 120 or so
• there are problems with theano and maybe sphinx - let's remove hep_ml.nnets, and move sphinx to dev-deps
• packaging: sure, let's update. My recommendation is hatch, minimal and works flawlessly

I suggest smaller PRs, then I'll help if necessary.

[jonas-eschle]

That's exactly what I had in mind (including 120 line-length, the sweet spot to me), going ahead

[jonas-eschle]

To track the changes, all are gonna be staged and you can merge one after the other

• upgrade to pyproject toml and Python 3.9, remove nnets, adding a changelog (Upgrade 1: upgrade to pyproject.toml and python 3.9+ #92 )
• Update deps: increase minimum scikit-learn version, move to pytest (nosetests is not maintained anymore really) (Upgrade 2: Update dependencies, pytest, remove old tests #93)
• Ruff formatting and pre-commit, Upgrade CI tests to add notebooks (should fail) (Upgrade 3: Add ruff and pre-commit, test notebooks #94 )
• Update CD, use trusted publisher
• Upgrade docs
• Upgrade notebooks, move root_numpy to uproot

[jonas-eschle]

What do you think so far? Is this also what you had in mind?

The last three points maybe need a bit of input

• trusted publisher, is that fine?
• Docs are currently kinda part of the source code. I would suggest to use RTD to build all versions and delete the built files?
• notebooks need a bit of upgrade maybe. We should move to uproot, a ROOT independent data loader, root_numpy is deprecated and unmaintained, (but there's also REP in the deps, which kinda complicates everything). Any opinion?

[arogozhnikov]

I needed to make rebases on last PR, and not sure if I can modify your branch.
I've added you to collaborators, let's create branches in repo, that's more convenient.

What do you think so far? Is this also what you had in mind?

that's super-awesome, thank you!

What I've changed: some utils were replaced ruff rules, and I've deleted notebook-stripper.
Reason: notebooks' outputs aren't good for git , sure - but we should make it super-easy to just open notebook on github and read how it works without asking users to install package. So let git/github suffer for user's convenience.

trusted publisher, is that fine?

It actually auto-deploys, and I even expect that after just-merged-PRs it would (correctly) auto-deploy to pypi on tag creation.
Sure, I can setup trusted publisher if that helps.

I would suggest to use RTD to build all versions and delete the built files?

"deployed" docs are currently just an additional branch.

I personally found using changelogs (which you already started 🙏 ) / release comments and remarks like 'added in v3.4' more helpful than RTD-style revisions of docs. If you have strong preference to RTD - fine with that too.

notebooks need a bit of upgrade maybe. We should move to uproot, a ROOT independent data loader, root_numpy is deprecated and unmaintained, (but there's also REP in the deps, which kinda complicates everything). Any opinion?

Great suggestion, let's move to uproot.

REP ... I think notebook uses only basic things that can be rewritten to plain sklearn / matplotlib? We should remove REP as a requirement anyway.

[arogozhnikov]

Also

• add CI workflow to run ruff check and ruff format

[jonas-eschle]

let's create branches in repo, that's more convenient.

Sure, thanks a lot!

... and I've deleted notebook-stripper. Reason: notebooks' outputs aren't good for git , sure - but we should make it super-easy to just open notebook on github and read how it works without asking users to install package. So let git/github suffer for user's convenience.

I generally fully agree! I think there are two problems two, one is the git as you mentioned (especially with many images, it can just blow up), but the other one is that the content may be out of date. Nothing guarantees that the notebook was created with the newest version.
My idea: why not include them in the docs using jupyter sphinx (i.e. it's built with the docs)? And add a button that opens binder directly for users to try out?

trusted publisher, is that fine?

It actually auto-deploys, and I even expect that after just-merged-PRs it would (correctly) auto-deploy to pypi on tag creation. Sure, I can setup trusted publisher if that helps.

It currently still should work, agree, so we can keep as is (push back in priorities). Personally, I've changed to making an explicit release, but tag is also fine. What about using VCS (i.e. last tag, hatch-vcs does that) then to automatically version? This way we don't need to update the version ourself

"deployed" docs are currently just an additional branch.

I personally found using changelogs (which you already started 🙏 ) / release comments and remarks like 'added in v3.4' more helpful than RTD-style revisions of docs. If you have strong preference to RTD - fine with that too.

Fully agree overall, I think the RTD can simply be additional and allows to go back to a specific version if needed (plus it can build the docs on PRs, an additional check). I'll try it out, low priority though, it shouldn't change any of the existing setup, just be additional.

REP ... I think notebook uses only basic things that can be rewritten to plain sklearn / matplotlib? We should remove REP as a requirement anyway.

Agree!

Also

• add CI workflow to run ruff check and ruff format

What about https://pre-commit.ci/? This allows to run the whole pre-commit, not just ruff, and apply auto-fixes

[arogozhnikov]

My idea: why not include them in the docs using jupyter sphinx (i.e. it's built with the docs)? And add a button that opens binder directly for users to try out?

it's just nice to have a notebook rendered on github, then just copy / download repo and have this all in front of you while you tinker with it. Fine with other options too.

Personally, I've changed to making an explicit release, but tag is also fine.

🙏 yeah, I meant release.

What about https://pre-commit.ci/? This allows to run the whole pre-commit, not just ruff, and apply auto-fixes

fewer services - less things to care/fix about, specially those that can modify code. Just pre-commit run --all-files in CI.

[arogozhnikov]

And add a button that opens binder directly for users to try out?

binder or colab (whichever) would be very nice. I'd still prefer to start with a notebook that already has plots :)

[jonas-eschle]

it's just nice to have a notebook rendered on github, then just copy / download repo and have this all in front of you while you tinker with it. Fine with other options too.

Agree as well. It's a matter of how the user gets exposed to it, if they land directly on the docs such as here, they can also just download it there.

binder or colab (whichever) would be very nice. I'd still prefer to start with a notebook that already has plots :)

I guess I don't mind and would rather prefer an empty notebook over a possibly outdated one, but having the content in doesn't do any harm (git will eat it).
One thing I am not sure about though: when should they be rerun and commited? "Every time", i.e. as part of pre-commit basically? For releases? Just to prevent notebooks from not having been updated in years (they won't break, they're added to the CI, but for the display). Maybe you've got some ideas? (otherwise it's "whatever it is", also fine with me)

fewer services - less things to care/fix about, specially those that can modify code. Just pre-commit run --all-files in CI.

But this won't autoformat or fix, only check, right?

There is an action which runs the fixes and commits them back (we can run only the safe fixes to not introduce anything else), but it's deprecated in favor of pre-commit.ci. Would you prefer only to check and the user needs to run pre-commit locally to fix instead of auto-fix and commit (as it's a separate commit, worst case we could always revert -- never had issue though)

[arogozhnikov]

But this won't autoformat or fix, only check, right?

correct. We can give a try to pre-commit.ci - I now see it is maintained by asottile.
Still, I have low expectations from external services for years-long support - case you're specifically trying to address.

(for example, all websites I relied on when starting project are currently either not working, or changed API, or changed ToS)

Just to prevent notebooks from not having been updated in years

'Rerunning notebooks' per se isn't challenging - this could be a simple CLI tool, but fact that sometimes notebooks must be updated (e.g. to fix that dependency is not maintained) - that's much more work that's not automatable right now.

Would be great if we could make this a part of release procedure.

[jonas-eschle]

correct. We can give a try to pre-commit.ci - I now see it is maintained by asottile.

Let's try it then. Can you set it up? It's a simple activation on https://results.pre-commit.ci/. I can't as I am not the owner of the repo. Already done as I see, thanks!

Still, I have low expectations from external services for years-long support - case you're specifically trying to address.
(for example, all websites I relied on when starting project are currently either not working, or changed API, or changed ToS)

Ahhh I know the struggle, yes. scrutiny, travis-ci, landscape and whatevernot, seen them rise, seen them die and change too, I agree! I think pre-commit is inherently different, one of the more stable services, I've been using it since years (but I know...) and changing is easy enough to github actions if needed (well, as long as github actions... let's just hope for the moment)

'Rerunning notebooks' per se isn't challenging - this could be a simple CLI tool, but fact that sometimes notebooks must be updated (e.g. to fix that dependency is not maintained) - that's much more work that's not automatable right now.

Yes, I just thought on the rerunning. That could clutter git though with images over time (but alas, not a main priority, it's fine as it is, and if people want an up-to-date version, they can run it themselves or in binder...

Would be great if we could make this a part of release procedure.

Yes, I think especially for the docs to have it automated.

I've openend #96 it's just a minor update to the notebook (and rerun all of them).

Not sure if I find the time for the docs, maybe we can just make a new release after #96, what do you think?

For the release (great you have a wiki! A bit outdated though, I updated a bit: https://github.com/arogozhnikov/hep_ml/wiki/Release-checklist), I can gladly to this (if you want), just to see if I can do it.

Version 0.8.0?

And since everything is handled in the CD (and tests in CI), we don't need to do anything manually, right? Except pushing the docs? (which we could just add to the cd as well?)

[arogozhnikov]

I've openend #96 it's just a minor update to the notebook (and rerun all of them).

thank you! merged

Not sure if I find the time for the docs, maybe we can just make a new release after #96, what do you think?

sure, let's try! I expect it to work, just checked contents of .tar.gz in build - it looks sane, contains .py files and includes new _version.py

Version 0.8.0?

👍

And since everything is handled in the CD (and tests in CI), we don't need to do anything manually, right?

yes, just create a release

[jonas-eschle]

yes, just create a release

Done! (after a botched attempt :P) and available: https://pypi.org/project/hep-ml/

[arogozhnikov]

awesome 🥳 , thank you so much!