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

Remove python warnings #2421

Merged
merged 4 commits into from Nov 23, 2023
Merged

Remove python warnings #2421

merged 4 commits into from Nov 23, 2023

Conversation

LukasNickel
Copy link
Member

@LukasNickel
Copy link
Member Author

Ah sorry, apparently I did not have my pre-commit set up

Copy link
Member

@morcuended morcuended left a comment

Choose a reason for hiding this comment

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

thanks @LukasNickel, just a typo in the changelog

docs/changes/2421.bugfix.rst Outdated Show resolved Hide resolved
@kosack
Copy link
Contributor

kosack commented Oct 31, 2023

Maybe it's not for this PR, but the overall behavior here should be updated to be more in line with how we do the DL1 and DL2 steps. In ctapipe-process, we have methods like should_compute_dl1() that determine if the DL0→DL1 transition should happen or not depending on the existence of pre-existing DL1 or the user specifying it to be recomputed, and same with should_compute_dl2(). It would make more sense to have the same thing for DL0: if DL0 already exists, there should be no need for a warning, but the user should have the option to force re-computing it if and only if R1 also exists.

That way all transitions are handled in the same way, and you only need to deal with the case if the user asks explicitly for a transformation to be re-computed, but the previous data level doesn't exist in the input. And in that case it's not even a warning, but an error that should stop processing.

- Use logging of the component
- No R1 is expected, so its just debug level
- No DL0 is not expected, so keep it as warning
Copy link

codecov bot commented Nov 23, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a2b6072) 92.48% compared to head (de7cb58) 92.47%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2421      +/-   ##
==========================================
- Coverage   92.48%   92.47%   -0.01%     
==========================================
  Files         234      234              
  Lines       19831    19819      -12     
==========================================
- Hits        18340    18328      -12     
  Misses       1491     1491              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@maxnoe maxnoe merged commit 26d3095 into main Nov 23, 2023
14 checks passed
@maxnoe maxnoe deleted the remove_warnings branch November 23, 2023 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CameraCalibrator warns in case no R1 data is present
5 participants