-
Notifications
You must be signed in to change notification settings - Fork 35
Add initial pandas 3 support #844
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
18279b1 to
7f8abbc
Compare
7f8abbc to
518650f
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #844 +/- ##
==========================================
- Coverage 83.46% 83.45% -0.01%
==========================================
Files 49 49
Lines 7293 7291 -2
==========================================
- Hits 6087 6085 -2
Misses 1206 1206 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds initial support for pandas 3 by addressing compatibility issues in test assertions and data type handling. The changes primarily focus on making the codebase work with both pandas 2 and pandas 3 by relaxing strict type checking in tests and simplifying data conversion logic.
Key Changes:
- Updated test assertions to use
check_index_type=False,check_dtype=False, andcheck_column_type=Falseparameters to accommodate pandas 3's different type behaviors - Refactored
compute_date_from_daysfunction to use more explicit logic and avoid deprecated pandas behaviors - Simplified
convert_dframe_date_to_strby removinginfer_objectsandfuture.no_silent_downcastingcontext manager
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_ofmvol2csv.py | Added check_index_type=False to test assertions to handle index type differences in pandas 3 |
| tests/test_fmuobs_writers.py | Added check_dtype=False to accommodate dtype variations between pandas versions |
| tests/test_fmuobs_parsers.py | Added check_dtype=False to test assertions for type compatibility |
| tests/test_fmuobs.py | Added check_dtype=False to yaml roundtrip test assertions |
| tests/test_csv2ofmvol.py | Added check_index_type=False to handle index type differences |
| tests/test_check_swatinit.py | Added check_column_type=False for column type compatibility |
| src/subscript/fmuobs/writers.py | Removed deprecated infer_objects call and option context manager |
| src/subscript/fmuobs/parsers.py | Refactored date computation logic to be more explicit and removed numpy import |
| pyproject.toml | Updated pandas dependency to require version 2 or higher |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Returns: | ||
| pd.DataFrame. DATE column is always of type datetime64 | ||
| pd.DataFrame. DATE column is always datetime-like | ||
| (datetime64 unit depends on pandas) |
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docstring update mentions that 'datetime64 unit depends on pandas' but this is imprecise. Consider clarifying whether this refers to the resolution (e.g., ns vs. us) and documenting which pandas versions use which units, or stating that the specific unit should not be relied upon by consumers of this function.
| (datetime64 unit depends on pandas) | |
| (typically ``datetime64[ns]``; the exact time unit/resolution is | |
| determined by pandas and should not be relied upon by callers). |
| return dframe | ||
|
|
||
| start = pd.to_datetime(starttime) | ||
| computed_dates = start + pd.to_timedelta(dframe["DAYS"], unit="D") |
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The refactored logic computes dates for all rows (including those with NaN in DAYS column), which differs from the original behavior that only computed dates for date_needed_rows. While pandas handles NaN gracefully in timedelta operations, this change processes more data than necessary. Consider adding .where(dframe['DAYS'].notna()) to maintain the original selective computation behavior.
| computed_dates = start + pd.to_timedelta(dframe["DAYS"], unit="D") | |
| computed_dates = (start + pd.to_timedelta(dframe["DAYS"], unit="D")).where( | |
| dframe["DAYS"].notna() | |
| ) |
No description provided.