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

Add a note to CameraGeometry docs about from_name() #2485

Merged
merged 4 commits into from
Mar 25, 2024

Conversation

kosack
Copy link
Contributor

@kosack kosack commented Dec 19, 2023

Note that from_name() is not guaranteed to correspond with event data.

Closes #2484

@kosack kosack added the documentation-only Label that will ensure code tests are skipped label Dec 19, 2023
@maxnoe
Copy link
Member

maxnoe commented Dec 19, 2023

camera_types is a set, you can't Index it

@maxnoe
Copy link
Member

maxnoe commented Jan 4, 2024

Can we (also?) raise an actual code warning?

i.e.:

warnings.warn(".from_name methods use pre-defined data that is likely different from the data being analyzed. Access instrument information via the SubarrayDescription instead.")

Tobychev
Tobychev previously approved these changes Jan 8, 2024
Tobychev
Tobychev previously approved these changes Jan 24, 2024
@maxnoe
Copy link
Member

maxnoe commented Jan 24, 2024

The docs failure here is real and due to the changes made

@maxnoe maxnoe modified the milestones: 0.22.0, v0.21.0 Mar 22, 2024
@maxnoe
Copy link
Member

maxnoe commented Mar 22, 2024

@kosack, I added the actual warning now to those methods and made it fail the CI if it occurs unexpectedly.

It's your PR, so you cannot really approve, but please have a look.

@Tobychev please review again :)

@maxnoe maxnoe removed the documentation-only Label that will ensure code tests are skipped label Mar 22, 2024
@maxnoe maxnoe force-pushed the fix_camerageom_docstring branch from af06fa7 to a9cf92f Compare March 22, 2024 12:29
Copy link

codecov bot commented Mar 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.54%. Comparing base (163b543) to head (a9cf92f).
Report is 11 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2485   +/-   ##
=======================================
  Coverage   92.53%   92.54%           
=======================================
  Files         235      236    +1     
  Lines       20063    20089   +26     
=======================================
+ Hits        18565    18591   +26     
  Misses       1498     1498           

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

@maxnoe maxnoe requested a review from Tobychev March 22, 2024 12:45
@kosack kosack merged commit 3e29788 into main Mar 25, 2024
15 checks passed
@kosack kosack deleted the fix_camerageom_docstring branch March 25, 2024 11:51
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.

Inconsistent LSTCam pixels ordering when importing using CameraGeometry.from_name
3 participants