icecube / flarestack

Unbinned likelihood analysis code for astroparticle physics datasets
https://flarestack.readthedocs.io/en/latest/?badge=latest
MIT License
8 stars 7 forks source link

Flarestack incompatible with latest version of black #251

Closed robertdstein closed 1 year ago

robertdstein commented 1 year ago

Describe the bug The newest version of black (v23) has introduced some new rule for removing blank lines. The upshot is that the pylint test, and therefore CI, will now fail unless somebody runs a new version of black on the code.

To Reproduce See e.g https://github.com/icecube/flarestack/actions/runs/4188047327/jobs/7258687765

mlincett commented 1 year ago

It could be convenient if the version of black run by the workflow could match the one specified by the flarestack dependencies.

I have nothing against running black v23 on all the code, but I would exclude the analyses subfolder.

mlincett commented 1 year ago

It appears from #259 that the test does not fail when it should.

mlincett commented 1 year ago

After #260 the Lint test fails when it should, however the Lint test is not "required" for merging, and nothing prevented dependabot/automerge to upgrade the version of black on the main branch.

Not sure whether we want to make Lint required for merging, though.

robertdstein commented 1 year ago

I think we should require Lint for merging too

mlincett commented 1 year ago

I think we should require Lint for merging too

It seems simpler than checking for non-required test to succeed before automerging, at least from my brief investigation.

Although I do not have access to the repo settings, maybe you can enable me?

mlincett commented 1 year ago

From the release notes of Black 23.3.0:

This release fixes a longstanding confusing behavior in Black's GitHub action, where the version of the action did not determine the version of Black being run (issue #3382). In addition, there is a small bug fix around imports and a number of improvements to the preview style.

So maybe we can switch back to the action.