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

Fix mt metadata issue 173 #174

Merged
merged 28 commits into from
Dec 15, 2023
Merged

Fix mt metadata issue 173 #174

merged 28 commits into from
Dec 15, 2023

Conversation

kkappler
Copy link
Collaborator

@kkappler kkappler commented Nov 24, 2023

Added some organization to the channel response filter.

see mt_metadata fix_issue_173
for more detail

- Added a method get_response_correction_operation_and_units which tries to determine if the calibration should be applied
as a multiplication or division operation, and identifies the calibration operation based on this.
@codecov-commenter
Copy link

codecov-commenter commented Nov 24, 2023

Codecov Report

Attention: 51 lines in your changes are missing coverage. Please review.

Comparison is base (401e360) 58.92% compared to head (28c7495) 58.95%.

Files Patch % Lines
mth5/timeseries/ts_filters.py 31.81% 15 Missing ⚠️
mth5/timeseries/channel_ts.py 78.78% 14 Missing ⚠️
mth5/timeseries/run_ts.py 60.00% 4 Missing ⚠️
mth5/io/phoenix/readers/receiver_metadata.py 0.00% 3 Missing ⚠️
tests/io/phoenix/test_read_phoenix_continuous.py 0.00% 3 Missing ⚠️
tests/io/phoenix/test_read_phoenix_segmented.py 0.00% 3 Missing ⚠️
mth5/groups/filters.py 0.00% 2 Missing ⚠️
mth5/io/zen/zen.py 0.00% 2 Missing ⚠️
tests/io/zen/test_z3d.py 0.00% 2 Missing ⚠️
mth5/groups/run.py 50.00% 1 Missing ⚠️
... and 2 more

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #174      +/-   ##
==========================================
+ Coverage   58.92%   58.95%   +0.03%     
==========================================
  Files         138      138              
  Lines       14928    14997      +69     
==========================================
+ Hits         8796     8842      +46     
- Misses       6132     6155      +23     
Flag Coverage Δ
tests 58.95% <69.09%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

- filter was being added to the filters list via append, but that skips the validator
- add filter via direct assignment of list instead, this calls the setter, which in turn calls the validator
- in future, if we want to append, we may want a decorator that calls the validator
- make unit checks in remove_instrument_response use mt_metadata.utils.units
Create access point for complex_response kwargs in remove_instrument_response,
specifically: include_decimation, include_time_delay
Implement channel_response_filter.get_list_of_filters_to_remove
where response removal occurs.
- only flip the filters that were removed
- add some doc about units, remove reference to "hack"
- use get_unit_object, and place full name of unit in channel_ts and channel_metadata
- removed (unused) optional argument "operation" in remove_instrument_response
remove references to filter correction operation
@kkappler kkappler requested a review from kujaku11 December 12, 2023 19:58
Copy link
Owner

@kujaku11 kujaku11 left a comment

Choose a reason for hiding this comment

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

A good logic fix on filters. Tests are updated alone with examples

@kujaku11 kujaku11 merged commit 27d98d7 into master Dec 15, 2023
10 checks passed
@kujaku11 kujaku11 deleted the fix_mt_metadata_issue_173 branch December 15, 2023 23:26
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.

3 participants