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

Updating bugs and Adding functionality #226

Merged
merged 48 commits into from
Aug 16, 2024
Merged

Updating bugs and Adding functionality #226

merged 48 commits into from
Aug 16, 2024

Conversation

kujaku11
Copy link
Owner

@kujaku11 kujaku11 commented Jul 12, 2024

Fix issues #217, #219, #223, #220

Add functionality and minor bug fixes

  • update to work with numpy 2.0

kkappler and others added 30 commits June 19, 2024 09:54
- AttributeError: `np.float_` was removed in the NumPy 2.0 release. Use `np.float64` instead.
- also, add %%time back into ipynb, this was not an issue
- Is it possible that github is using common storage for the tests?
- I'm getting what look like synchronous access errors maybe related to
  multiple processes trying to access same hdf5?
- KeyError: "Unable to synchronously open object (object 'channel_summary' doesn't exist)"
- while working on issue #219, using the syntethic
examples for some snippets, the way survey was hokey
- now it is less hokey
- working with v0.1.0 files, updating survey metadata was triggering
setting of stations df
- this df may be empty
- added a check for len(df) before trying to assign start/end
- working with v0.1.0 files, updating survey metadata was triggering
  operations on stations df
- this df may be empty
- added a check for len(df) before trying to assign metadata from df
- also comment out an assert that fails for files with no survey name
- added a TODO for that
- these tests actually relate to #209
- added some doc as well
kkappler and others added 10 commits July 12, 2024 15:22
- a sort of continuation of issue #209, but a new fork from patches as that branch is already merged
- add MultivariateLabelScheme() class to manage how we label multivariate channels
- add MultivariateDataset() class to wrap the MV xarray
- add some tests
- tests could be better organized if address issue #227
Copy link
Collaborator

Choose a reason for hiding this comment

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

NIT: These changes do expand the tests, but a better test_fcs could be built -- one that tests acts a validator for an already built mth5 -- There are also some repeated numeric values that could be made into a fixture or helper class such that they are only declared once (e.g. expected_sr_decimation_level = 0.015380859375)

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 70.72243% with 77 lines in your changes missing coverage. Please review.

Project coverage is 61.65%. Comparing base (2f7784b) to head (a669826).
Report is 45 commits behind head on master.

Files Patch % Lines
mth5/io/zen/z3d_metadata.py 0.00% 24 Missing ⚠️
mth5/utils/extract_subset_mth5.py 75.94% 19 Missing ⚠️
mth5/utils/helpers.py 56.00% 11 Missing ⚠️
mth5/data/make_mth5_from_asc.py 61.11% 7 Missing ⚠️
mth5/timeseries/scipy_filters.py 40.00% 6 Missing ⚠️
tests/utils/test_issue_219.py 92.72% 4 Missing ⚠️
mth5/groups/station.py 57.14% 3 Missing ⚠️
mth5/utils/fc_tools.py 50.00% 3 Missing ⚠️

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

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #226      +/-   ##
==========================================
+ Coverage   61.28%   61.65%   +0.37%     
==========================================
  Files         145      149       +4     
  Lines       16124    16411     +287     
==========================================
+ Hits         9882    10119     +237     
- Misses       6242     6292      +50     
Flag Coverage Δ
tests 61.65% <70.72%> (+0.37%) ⬆️

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

added some doc strings here

Copy link
Collaborator

@kkappler kkappler left a comment

Choose a reason for hiding this comment

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

Lots of small improvements here -- tests are passing, what's not to like? Approved.

Add some more multivariate functionality


There may be some more methods needed for issue #209, but these tools seem to do most of what is needed, and tests are passing so merging into `patches`.
- added a try/except
- if exception encountered, message telling user what FCs are there, and suggesting to add the requested group
- added a test for the exception
Minor multivariate updates

Probably should have pushed this direct to patches
@kujaku11 kujaku11 merged commit b245d16 into master Aug 16, 2024
8 checks passed
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