Skip to content

Handle nans in data#125

Merged
henrikjacobsenfys merged 5 commits into
developfrom
handle-nans-in-data
Mar 11, 2026
Merged

Handle nans in data#125
henrikjacobsenfys merged 5 commits into
developfrom
handle-nans-in-data

Conversation

@henrikjacobsenfys

@henrikjacobsenfys henrikjacobsenfys commented Mar 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Closing #119
Closing #118

@henrikjacobsenfys henrikjacobsenfys added [scope] bug Bug report or fix (major.minor.PATCH) [priority] high Should be prioritized soon labels Mar 9, 2026
@codecov

codecov Bot commented Mar 9, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (develop@dadf1f1). Learn more about missing BASE report.

Additional details and impacted files 
@@            Coverage Diff             @@
##             develop     #125   +/-   ##
==========================================
  Coverage           ?   97.77%           
==========================================
  Files              ?       36           
  Lines              ?     2334           
  Branches           ?      388           
==========================================
  Hits               ?     2282           
  Misses             ?       32           
  Partials           ?       20
Flag Coverage Δ
integration 0.00% <0.00%> (?)
unittests 97.77% <100.00%> (?)

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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

rozyczko
rozyczko approved these changes Mar 10, 2026
View reviewed changes

@rozyczko rozyczko 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.

Looks very good! Just a note about the possibility of adding more unit test coverage.

Comment thread tests/unit/easydynamics/experiment/test_experiment.py
Comment on lines +420 to +421
invalid_data.data.variances[Q_index] = 0 # Set variances to zero

@rozyczko rozyczko Mar 10, 2026

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.

This sets all variances for Q_index=0 to zero. It means they all pass the isfinite check but then fail the e == 0 check. This is correct, but it would be stronger to also test the case where only some variances are zero while others have NaN/Inf. This would verify that the filtering happens first and the zero-check second.

@henrikjacobsenfys henrikjacobsenfys Mar 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks, made one of the variances NaN :)

@henrikjacobsenfys henrikjacobsenfys merged commit e28e3c0 into develop Mar 11, 2026
34 checks passed
@henrikjacobsenfys henrikjacobsenfys deleted the handle-nans-in-data branch March 25, 2026 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rozyczko rozyczko rozyczko approved these changes

Assignees

No one assigned

Labels

[priority] high Should be prioritized soon [scope] bug Bug report or fix (major.minor.PATCH)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@henrikjacobsenfys @rozyczko