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

Kmod scripts new branch #463

Merged
merged 35 commits into from
Nov 13, 2024
Merged

Kmod scripts new branch #463

merged 35 commits into from
Nov 13, 2024

Conversation

fscarlier
Copy link
Contributor

@fscarlier fscarlier commented Sep 25, 2024

Scripts are separate in omc3 for now, but could be added into a separate directory if needed.

TODO (jdilly):

  • Import also kmod_betastar_z.tfs
  • Better doc for what the path is (dir or file)
  • Lumi imbalance: also ip1 / ip2 or ip5 / ip2
  • Add to check_corrections -> TBD later in another PR

@fscarlier fscarlier requested a review from a team as a code owner September 25, 2024 09:38
@JoschD JoschD assigned fscarlier and unassigned JoschD Sep 25, 2024
@JoschD JoschD self-requested a review September 25, 2024 09:44
@JoschD JoschD added Type: Feature A (suggetion for a) new feature or enhancement in functionality. Domain: GUI Closely related to GUI and/or needs GUI changes as well. Status: In Progress Currently being worked on. Priority: Medium Work on this. Estimate: Normal Straightforward, but might require some time. Probably needs additional tests. labels Sep 25, 2024
omc3/kmod_averages.py Outdated Show resolved Hide resolved
omc3/kmod_averages.py Outdated Show resolved Hide resolved
omc3/kmod_averages.py Outdated Show resolved Hide resolved
omc3/kmod_lumi_imbalance.py Outdated Show resolved Hide resolved
omc3/kmod_lumi_imbalance.py Outdated Show resolved Hide resolved
@JoschD JoschD requested a review from a team September 26, 2024 12:29
@JoschD JoschD added Status: On Hold Work currently stopped, but not for blocking reasons. Status: Review Needed Work currently stopped, untils someone else reviews it. and removed Status: In Progress Currently being worked on. labels Sep 26, 2024
JoschD
JoschD previously approved these changes Nov 11, 2024
Copy link
Member

@fsoubelet fsoubelet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I like it much more with just a few clean scripts / modules and one main entrypoint.

Plenty of little things here and there, most of it surface level. As usual I'm not too manic about it and they should be considered suggestions more than demands, obviously.

There are a few questions in the lot that I'd like to see answered as I might have misunderstood in a few places.

I expect this can easily be merged today if you get on it :)

CHANGELOG.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
omc3/kmod_importer.py Outdated Show resolved Hide resolved
omc3/kmod_importer.py Outdated Show resolved Hide resolved
tests/unit/test_kmod_import.py Outdated Show resolved Hide resolved
tests/unit/test_kmod_importer.py Outdated Show resolved Hide resolved
tests/unit/test_kmod_importer.py Outdated Show resolved Hide resolved
tests/unit/test_kmod_lumi_imbalance.py Outdated Show resolved Hide resolved
tests/unit/test_kmod_lumi_imbalance.py Outdated Show resolved Hide resolved
Copy link
Member

@fsoubelet fsoubelet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only requesting changes to fix the documentation build, otherwise all good!

doc/entrypoints/scripts.rst Outdated Show resolved Hide resolved
omc3/scripts/kmod_lumi_imbalance.py Show resolved Hide resolved
omc3/scripts/kmod_lumi_imbalance.py Show resolved Hide resolved
omc3/scripts/kmod_lumi_imbalance.py Show resolved Hide resolved
omc3/scripts/kmod_lumi_imbalance.py Outdated Show resolved Hide resolved
tests/unit/test_conftest.py Outdated Show resolved Hide resolved
Copy link

codeclimate bot commented Nov 13, 2024

Code Climate has analyzed commit a1878a0 and detected 9 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 9

The test coverage on the diff in this pull request is 91.4% (50% is the threshold).

This pull request will bring the total coverage in the repository to 85.9% (0.0% change).

View more on Code Climate.

@JoschD JoschD merged commit 19f7e86 into master Nov 13, 2024
37 checks passed
@JoschD JoschD deleted the kmod_scripts_new_branch branch November 13, 2024 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: GUI Closely related to GUI and/or needs GUI changes as well. Estimate: Normal Straightforward, but might require some time. Probably needs additional tests. Priority: Medium Work on this. Status: In Progress Currently being worked on. Status: Review Needed Work currently stopped, untils someone else reviews it. Type: Feature A (suggetion for a) new feature or enhancement in functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants