Skip to content

Conversation

@d-chambers
Copy link
Contributor

Description

This PR modifies the time/distance decimation check for reading neubrex DAS files. Previously, we simply raise an exception if the DistanceDecimationFilter or the TimeDecimationFilter attributes are non-zero. This is because I was not sure if this is a binary flag indicating a low-pass anti-aliasing filter was applied (to either the time or space dimension) or if it is an integer value indicating the time/distance coordinate was decimated. For example, a DistanceDecimationFilter=2 could mean every other channel was discarded.

It appears that some files (eg the forge neubrex das files) have a value of 1, so it is probably a binary flag. In this case, I don't think there is anything else for DASCore to do except add this info to the attrs.

This PR just allows DistanceDecimationFilter and TimeDecimationFilter to be either 1 or 0 and adds distance_decimation_filter and time_decimation_filter to the patch attrs.

@arturguzik, if you can confirm my intrepretation of the TimeDecimationFilter and DistanceDecimationFilter parameters that would be useful. Thanks!

Checklist

I have (if applicable):

  • referenced the GitHub issue this PR closes.
  • documented the new feature with docstrings or appropriate doc page.
  • included a test. See testing guidelines.
  • your name has been added to the contributors page (docs/contributors.md).
  • added the "ready_for_review" tag once the PR is ready to be reviewed.

@d-chambers d-chambers added the IO Work for reading/writing different formats label Dec 4, 2024
@codecov
Copy link

codecov bot commented Dec 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.85%. Comparing base (ad2ecb5) to head (be5b9c8).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #465   +/-   ##
=======================================
  Coverage   99.85%   99.85%           
=======================================
  Files         115      115           
  Lines        9372     9374    +2     
=======================================
+ Hits         9358     9360    +2     
  Misses         14       14           
Flag Coverage Δ
unittests 99.85% <100.00%> (+<0.01%) ⬆️

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.

@d-chambers
Copy link
Contributor Author

I also updated the mamba setup to actually use the correct version of python (it appears the latest version was being used before).

@d-chambers d-chambers merged commit 3001b76 into master Dec 6, 2024
15 checks passed
@d-chambers d-chambers deleted the neubrex_forge branch December 6, 2024 23:41
d-chambers added a commit that referenced this pull request Dec 21, 2024
* fix_463

* neubrex decimation check (#465)

- also fixes CI mamba setup.

* segyio dependency issue

note made next to install instructions

* clarify fetch in tutorials (#468)

* fix_463

---------

Co-authored-by: eileenrmartin <[email protected]>
Co-authored-by: Ahmad Tourei <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

IO Work for reading/writing different formats

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants