Skip to content

🐛 Small fix for bug in compensation function.#39

Merged
mbuttner merged 2 commits into
scverse:mainfrom
ricomnl:main
Jun 8, 2023
Merged

🐛 Small fix for bug in compensation function.#39
mbuttner merged 2 commits into
scverse:mainfrom
ricomnl:main

Conversation

@ricomnl
Copy link
Copy Markdown
Contributor

@ricomnl ricomnl commented May 30, 2023

This PR fixes an issue that arises when no indices between the compensation matrix columns and the adatas index (or channel column) are intersecting. Previously this was not caught because idx_in is None doesn't check if idx_in is empty and therefore not caught. I also added a check if X has changed after compensation and am logging a warning if it hasn't.

@mbuttner
Copy link
Copy Markdown
Collaborator

Hi @ricomnl
thanks for your PR! The code looks good so far, but the check failed: black-jupyter would reformat the _process_data.py a little. Can you run pre-commit, please?

@ricomnl
Copy link
Copy Markdown
Contributor Author

ricomnl commented Jun 3, 2023

just did :) thanks for the pointer

@mbuttner mbuttner changed the title Small fix for bug in compensation function. 🐛 Small fix for bug in compensation function. Jun 8, 2023
ricomnl added 2 commits June 8, 2023 09:38
…d check if X has changed after compensation and log a warning if not
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Merging #39 (b0cbb25) into main (a03ab1f) will decrease coverage by 0.06%.
The diff coverage is 66.66%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main      #39      +/-   ##
==========================================
- Coverage   80.46%   80.41%   -0.06%     
==========================================
  Files          10       10              
  Lines         384      388       +4     
==========================================
+ Hits          309      312       +3     
- Misses         75       76       +1     
Impacted Files Coverage Δ
pytometry/preprocessing/_process_data.py 65.00% <66.66%> (+0.41%) ⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@mbuttner mbuttner merged commit 6b37445 into scverse:main Jun 8, 2023
@mbuttner
Copy link
Copy Markdown
Collaborator

mbuttner commented Jun 8, 2023

Thanks for your contribution! I had to update the CI before I could merge the branch.

@ricomnl
Copy link
Copy Markdown
Contributor Author

ricomnl commented Jun 8, 2023

awesome thanks!

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