search

iris-hep / analysis-grand-challenge

Repository dedicated to AGC preparations & execution
https://agc.readthedocs.io
MIT License
24 stars 39 forks source link

fix: ruff linter errors #148

Closed eguiraud closed 1 year ago

eguiraud commented 1 year ago

@alexander-held there is only one error remaining (besides errors due to lines that are too long):

workshops/agctools2022/statistical-inference/exclusion/src/exclusion/interpolate.py:187:57: F821 Undefined name `constraintsDict`
Found 1 error.

Do you have any suggestion how to solve that one?

This is in preparation to the next PR which adds a ruff check to the CI.

alexander-held commented 1 year ago

I'm not sure who wrote that revised version of interpolate.py, perhaps it was @kratsg or @matthewfeickert? This line specifically is the problem:

https://github.com/iris-hep/analysis-grand-challenge/blob/859e3e0d6dd6529684086d3397b90d06f00f3682/workshops/agctools2022/statistical-inference/exclusion/src/exclusion/interpolate.py#L187

constraintsDict is never defined. In the HistFitter original version it is: https://github.com/histfitter/histfitter/blob/1492543dcb4df8364f9f6ef8e6d044d8a968db37/scripts/harvestToContours.py#L338-L347.

kratsg commented 1 year ago

I'm not sure who wrote that revised version of interpolate.py, perhaps it was @kratsg or @matthewfeickert?

I suspect @lukasheinrich was the one that did the modifications... unless it was @matthewfeickert . I didn't do this.

eguiraud commented 1 year ago

About the broken code in interpolate.py and visualize.py: both problems have been there since those files were first added in https://github.com/iris-hep/analysis-grand-challenge/commit/f037776ec8afdd5269fec8a3bca8778a8615dbf1 .

eguiraud commented 1 year ago

Rebased after amending the fixes to leave ; at the end of lines

alexander-held commented 1 year ago

Hi @lukasheinrich, @matthewfeickert, do either of you remember this exclusion contours script and can help us identify how to fix it?

matthewfeickert commented 1 year ago

Hi @lukasheinrich, @matthewfeickert, do either of you remember this exclusion contours script and can help us identify how to fix it?

Sorry today I'm a bit oversubscribed but @alexander-held @eguiraud please ping me if I haven't responded in 24 hours. (Also sorry for not seeing these notifications from a week ago! :grimacing:)

eguiraud commented 1 year ago

@matthewfeickert ping :smile: (I realize there are still 20 minutes before 24 hours have passed, but I was doing a pass over my open PRs now sorry!)

alexander-held commented 1 year ago

Hi @matthewfeickert, friendly ping here. If we cannot figure out what this was let's just go ahead and not touch those files.

edit: sorry for the double ping, this was not intended!

matthewfeickert commented 1 year ago

I'll look at this after lunch, but this indeed was me as this is what I threw together for a demo at the AGC Tools 2022 workshop which is https://pypi.org/project/exclusion/ (the source code currently lives on the feat/add-cookie branch of https://github.com/matthewfeickert/exclusion as I guess I never even finished that loop). Some of this stuff got ported from original workflows in HistFitter and as I was rushing I didn't clean some of the patterns that exist there. So I'll look at this and try to clean things, but I think the long and the short of it is that basically anything ruff wants to do should go in, unless you want to flip this out for just using exclusion from PyPI and then someone pushing me to maintain it (could probably find some interest in ATLAS SUSY groups I would hope).

eguiraud commented 1 year ago

Thanks @matthewfeickert , the latest commit removes the dead code.