-
Notifications
You must be signed in to change notification settings - Fork 272
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
Chunk interpolation to select calibration data #2634
base: main
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
3 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I am a bit stuck here with one of the tests. test_hdf5 is failing in pytest begause the data from the file is not loaded correctly. When i look in the test what x and y values the interpolators have i get some wrong values. However if i try to do the same outside of pytest it works. I used this code to test by myself:
Has anyone an idea what is wrong here? |
@ctoennis the problem you're experiencing with the test is because you have class attributes in the base class that are shared between the class instances, including the |
@mexanick Thanks, i got the issue fixed. |
@maxnoe Hi Max, could you give this another look? |
Simple interpolator for overlapping chunks of data. | ||
""" | ||
|
||
required_columns = frozenset(["start_time", "end_time"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an immutable type that you "mutate" further down in the code. Please don't do this, if you want to extend it with the value column, particular for an object, just make a copy and to the object attribute and modify that one.
""" | ||
|
||
required_columns = frozenset(["start_time", "end_time"]) | ||
expected_units = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected units can be different for different objects. Please ensure you can create two different ChunkInterpolators objects with different interpolation column and different units to it.
super().__init__(**kwargs) | ||
|
||
self._interpolators = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed before, the base class should be rather pure. An interface shouldn't prescribe private data layout.
- Move the _check_interpolators from MonitoringInterpolator to LinearInterpolator - Call _interpolate_chunk in directly __call__ of ChunkInterpolator
1e0e6d9
to
56afd67
Compare
This comment has been minimized.
This comment has been minimized.
1 similar comment
Analysis Details0 IssuesCoverage and DuplicationsProject ID: cta-observatory_ctapipe_AY52EYhuvuGcMFidNyUs |
I will need a method to select calibration data for the strar tracker. I made some slides to decribe how it is supposed to work:
https://docs.google.com/presentation/d/1oxIcYSQvGnU7IQYy3fGdcv0qXiLpvaXR9YtmnDesj4Y/edit?usp=sharing