np.dropna to include inf #535
Conversation
WalkthroughAdds an include_inf parameter (default True) to dropna and fillna in dascore/proc/basic.py so non-finite values (Inf/-Inf) are treated as nullish when enabled. Updates null/replacement masks accordingly. Tests refactored to module-level fixtures and add tests for Inf handling in dropna and fillna. Changes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #535 +/- ##
=======================================
Coverage 99.88% 99.88%
=======================================
Files 122 122
Lines 10040 10044 +4
=======================================
+ Hits 10028 10032 +4
Misses 12 12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
dascore/proc/basic.py (2)
511-512: Update the docstring to align with the new behavior.The docstring states that "nullish" is defined by
pandas.isnull, but withinclude_inf=True(the default), the function now usesnp.isfinitewhich has different behavior. The docstring should clarify this distinction.Apply this diff to update the docstring:
Notes ----- - "nullish" is defined by `pandas.isnull`. + When include_inf is False, "nullish" is defined by `pandas.isnull`. + When include_inf is True (default), "nullish" includes non-finite values + (NaN, inf, -inf) as determined by `numpy.isfinite`.
561-562: Update the docstring to align with the new behavior.Similar to
dropna, the docstring should clarify the distinction between the behavior wheninclude_infis True vs False.Apply this diff to update the docstring:
Notes ----- - "nullish" is defined by `pandas.isnull`. + When include_inf is False, "nullish" is defined by `pandas.isnull`. + When include_inf is True (default), "nullish" includes non-finite values + (NaN, inf, -inf) as determined by `numpy.isfinite`.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
dascore/proc/basic.py(6 hunks)tests/test_proc/test_basic.py(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
dascore/proc/basic.py (2)
dascore/core/patch.py (1)
data(213-215)dascore/core/coords.py (1)
data(606-608)
tests/test_proc/test_basic.py (3)
dascore/examples.py (3)
patch_with_null(115-130)get_example_patch(635-682)random_patch(29-111)tests/conftest.py (3)
random_patch(282-284)range_patch_3d(355-364)patch(375-377)dascore/proc/basic.py (3)
update(159-199)dropna(493-547)fillna(551-585)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: test_code (windows-latest, 3.11)
- GitHub Check: test_code (windows-latest, 3.12)
- GitHub Check: test_code (ubuntu-latest, 3.10)
- GitHub Check: test_code (windows-latest, 3.13)
- GitHub Check: test_code (macos-latest, 3.10)
- GitHub Check: test_code (ubuntu-latest, 3.11)
- GitHub Check: test_code (macos-latest, 3.11)
- GitHub Check: test_code (macos-latest, 3.13)
- GitHub Check: test_code (windows-latest, 3.10)
- GitHub Check: test_code (ubuntu-latest, 3.13)
- GitHub Check: test_code (ubuntu-latest, 3.12)
- GitHub Check: test_code_min_deps (windows-latest, 3.12)
- GitHub Check: test_code_min_deps (windows-latest, 3.13)
- GitHub Check: test_code_min_deps (ubuntu-latest, 3.12)
- GitHub Check: test_code_min_deps (macos-latest, 3.12)
- GitHub Check: test_code_min_deps (ubuntu-latest, 3.13)
- GitHub Check: test_code_min_deps (macos-latest, 3.13)
🔇 Additional comments (10)
dascore/proc/basic.py (4)
493-498: LGTM! Well-designed parameter signature.The addition of the
include_infparameter with a default value ofTrueprovides a clear and intuitive API for controlling whether infinite values should be treated as nullish values in the dropna operation.
530-533: LGTM! Clear conditional logic for handling finite vs null values.The implementation correctly uses
np.isfinitewheninclude_inf=Trueto catch NaN, inf, and -inf values, and falls back topd.isnullwheninclude_inf=Falsefor traditional null-only behavior.
551-551: LGTM! Consistent parameter signature with dropna.The
include_infparameter maintains consistency with thedropnafunction signature.
578-581: LGTM! Consistent implementation with dropna logic.The conditional logic correctly mirrors the approach used in
dropna, usingnp.isfinitefor comprehensive non-finite value detection andpd.isnullfor traditional null-only behavior.tests/test_proc/test_basic.py (6)
33-36: LGTM! Good use of session-scoped fixture.Using session scope for the
patch_with_nullfixture is efficient since this test data doesn't change and can be shared across all tests.
39-45: LGTM! Well-designed fixture for testing infinite values.The fixture creates a patch with infinite values by setting column 2 to
np.inf, providing good test coverage for the newinclude_inffunctionality.
48-56: LGTM! Appropriate use of class-scoped fixture.The class scope is appropriate for
patch_3d_with_nullsince it's specific to certain test classes and creates a more complex 3D test structure.
496-499: LGTM! Good test coverage for the new include_inf=True behavior.The test verifies that when
include_inf=True(the default), infinite values are properly dropped from the data.
501-504: LGTM! Good test coverage for include_inf=False behavior.The test ensures backward compatibility by verifying that when
include_inf=False, infinite values are preserved in the data.
530-534: LGTM! Good test coverage for fillna with infinite values.The test verifies that
fillnawithinclude_inf=Truecorrectly fills infinite values by checking that the count of zeros increases after the operation.
Description
This PR makes
drop_nainclude inf by default but this can be controlled with a newinclude_infargument. Same forfill_na.Checklist
I have (if applicable):
Summary by CodeRabbit
New Features
Behavior Change
Tests