FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix lag application for negative lags by tsalo · Pull Request #36 · physiopy/phys2denoise · GitHub

/ phys2denoise Public

Fix lag application for negative lags - #36

Open
tsalo wants to merge 7 commits into
physiopy:masterfrom
tsalo:rv-negative-lags
ConversationCommits (7)ChecksFiles changed
Open

Fix lag application for negative lags#36
tsalo wants to merge 7 commits into
physiopy:masterfrom
tsalo:rv-negative-lags

Conversation

tsalo commented Jun 28, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

Closes #35.

This PR corrects the calculation of negative lags.

Proposed Changes

  • Remove unused lags parameter in rv function.
  • Use the absolute lag when applying negative lags.
  • Replace uses of numpy.int (deprecated) with int.

Change Type

  • bugfix (+0.0.1)
  • minor (+0.1.0)
  • major (+1.0.0)
  • refactoring (no version update)
  • test (no version update)
  • infrastructure (no version update)
  • documentation (no version update)
  • other

Checklist before review

  • I added everything I wanted to add to this PR.
  • [Code or tests only] I wrote/updated the necessary docstrings.
  • [Code or tests only] I ran and passed tests locally.
  • [Documentation only] I built the docs locally.
  • My contribution is harmonious with the rest of the code: I'm not introducing repetitions.
  • My code respects the adopted style, especially linting conventions.
  • The title of this PR is explanatory on its own, enough to be understood as part of a changelog.
  • I added or indicated the right labels.
  • I added information regarding the timeline of completion for this PR.
  • Please, comment on my PR while it's a draft and give me feedback on the development!

smoia approved these changes Feb 19, 2023
View reviewed changes

smoia left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM!

smoia commented Feb 19, 2023

Copy link
Copy Markdown
Member

Actually, @tsalo, it's difficult for me to figure out which changes you wanted to stay - can you resolve the conflicts with master, please?

tsalo commented Mar 15, 2023

Copy link
Copy Markdown
Member Author

Sorry for the delay @smoia. I'll give that a try now.

tsalo changed the title Support negative lags in RV metric Fix lag application for negative lags Mar 15, 2023

tsalo commented Mar 15, 2023 •
edited
Loading

Copy link
Copy Markdown
Member Author

I ended up having to change a few things. Lags were removed from rv at some point, but the parameter was still there, so I removed the parameter.

Also, lags in apply_lags wasn't applying lags correctly, AFAICT. I think I've fixed that.

codecov Bot commented Mar 15, 2023

Copy link
Copy Markdown

Codecov Report

Merging #36 (f7c85b1) into master (24515ca) will not change coverage.
The diff coverage is 33.33%.

Additional details and impacted files 

@@           Coverage Diff           @@
##           master      #36   +/-   ##
=======================================
  Coverage   47.04%   47.04%           
=======================================
  Files           7        7           
  Lines         321      321           
=======================================
  Hits          151      151           
  Misses        170      170
Impacted Files Coverage Δ
phys2denoise/metrics/cardiac.py 94.73% <ø> (ø)
phys2denoise/metrics/utils.py 69.56% <0.00%> (ø)
phys2denoise/metrics/chest_belt.py 85.54% <100.00%> (ø)

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

smoia smoia approved these changes

Assignees

No one assigned

Labels

None yet

Projects

Physiopy
Status: PR needs review
phys2denoise
Status: PR to review

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support negative lags in RV calculation

2 participants

Back | FazBrowse Home | New Git URL