174 pylint alerts corrections #181

evidencebp · 2024-09-24T13:15:32Z

This PR takes care of issue "Pylint alerts corrections as part of intervention experiment" #174.

Each commit takes care of a single Pylint alert in a single file.
See the executed interventions.
The commit message explain the intervention and its justification (e.g., when narrowing an exception).

Most interventions are minor.
The larger ones are due to too-many-branches or too-many-statements.
In these cases I extracted methods, making the code shorter, easier to understand and modify.

When going over the PR please note:

I noticed some misalignments to black in the existing code. Since it was that way and I wanted to make small and focused interventions, I did not fix them.
I'm not familiar with the project logic. All interventions are small refactors that should keep the same logic. However, please consider my infamilarity.
I do not have the needed API keys. Hence, when running the code after the modification the flow did not reach the main path. I run the modified parts externally.
The conda installation I used did not install fastchat, used in eval\evaluation_prometheus.py . I tried to install it separately yet it did not work. Hence I could not run this file. However the intervention there was replacing Exception in ValueError, more focused. However, no other exception is expected there so this refactor should be safe.

Please tell me if there are unclear points or some other needed aspects.

Just adding a new line is not compatible with black. https://github.com/psf/black Instead extracted styles as a variable, similar to pages. Using the black playground I noted the few more changes are needed. I did not apply them to keep this modification focused.

See https://stackoverflow.com/questions/5627425/what-is-a-good-way-to-handle-exceptions-when-trying-to-read-a-file-in-python

Found unrelated black-style alerts. I did not apply them to keep the change focused.

See https://stackoverflow.com/questions/51338230/is-a-there-exception-if-you-ask-for-a-integer-and-do-not-get-an-integer

Noted black style alert not related to the change. I did not apply them to keep the change focused.

Method was too long, Extracted methods having a single responsibility, making the code easier to understand and shorter. There a black style alerts in the file. I din not fix them to keep the change focused.

frontend/demo_light/storm.py

shaoyijia · 2024-09-26T04:37:24Z

Hi @evidencebp, we recently have a very large update of our repo. Is it possible for you to rerun your algorithm to gets the edit and open a new PR?

evidencebp · 2024-09-26T11:01:40Z

Indeed a large update.
I merged the main into this branch.
Now there are only 3 files with interventions.
At list it makes the review easier...

Can you approve and merge now?

shaoyijia

Hope this helps your research!

evidencebp · 2024-09-27T09:15:05Z

Sure it will help!

In case that you would like to intervene yourself in other projects or have ideas for such, please tell me.
Of course, if you know others that might be interested, let them know too.

…s-corrections 174 pylint alerts corrections

evidencebp added 8 commits September 23, 2024 15:41

Fixed line-too-long

13ef9ed

Just adding a new line is not compatible with black. https://github.com/psf/black Instead extracted styles as a variable, similar to pages. Using the black playground I noted the few more changes are needed. I did not apply them to keep this modification focused.

A too long readable line in a comment

bf18d7d

Replaced Exception in IOError

1a41ef9

See https://stackoverflow.com/questions/5627425/what-is-a-good-way-to-handle-exceptions-when-trying-to-read-a-file-in-python

Extracted methods to reduce number of branches

226663a

Found unrelated black-style alerts. I did not apply them to keep the change focused.

Made line shorter

22463df

Change Exception to narrower ValueError

5855f31

See https://stackoverflow.com/questions/51338230/is-a-there-exception-if-you-ask-for-a-integer-and-do-not-get-an-integer

Extracted get_avg_results

2ee30af

Noted black style alert not related to the change. I did not apply them to keep the change focused.

Make compute_autoais shorter

19b4cf4

Method was too long, Extracted methods having a single responsibility, making the code easier to understand and shorter. There a black style alerts in the file. I din not fix them to keep the change focused.

evidencebp mentioned this pull request Sep 24, 2024

Pylint alerts corrections as part of intervention experiment #174

Closed

Yucheng-Jiang reviewed Sep 24, 2024

View reviewed changes

frontend/demo_light/storm.py Show resolved Hide resolved

Merge branch 'main' into 174-Pylint-alerts-corrections

5b7e00b

shaoyijia approved these changes Sep 27, 2024

View reviewed changes

shaoyijia merged commit 28ad542 into stanford-oval:main Sep 27, 2024

evidencebp mentioned this pull request Nov 10, 2024

Pylint alerts corrections as part of an intervention experiment SublimeText/PackageDev#401

Open

feldges pushed a commit to feldges/storm that referenced this pull request Dec 4, 2024

Merge pull request stanford-oval#181 from evidencebp/174-Pylint-alert…

735f547

…s-corrections 174 pylint alerts corrections

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

174 pylint alerts corrections #181

174 pylint alerts corrections #181

evidencebp commented Sep 24, 2024

shaoyijia commented Sep 26, 2024

evidencebp commented Sep 26, 2024

shaoyijia left a comment

evidencebp commented Sep 27, 2024

174 pylint alerts corrections #181

174 pylint alerts corrections #181

Conversation

evidencebp commented Sep 24, 2024

shaoyijia commented Sep 26, 2024

evidencebp commented Sep 26, 2024

shaoyijia left a comment

Choose a reason for hiding this comment

evidencebp commented Sep 27, 2024