-
Notifications
You must be signed in to change notification settings - Fork 35
Fix some warnings from tests #806
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
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #806 +/- ##
=======================================
Coverage 85.16% 85.16%
=======================================
Files 49 49
Lines 7544 7544
=======================================
Hits 6425 6425
Misses 1119 1119 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
5c39c35 to
1906fcb
Compare
3343cd0 to
29bd051
Compare
22bc552 to
559e20c
Compare
- Fixes:
"FutureWarning: Downcasting behavior in `replace` is deprecated and will be removed in a future version.
To retain the old behavior, explicitly call `result.infer_objects(copy=False)`.
To opt-in to the future behavior, set `pd.set_option('future.no_silent_downcasting', True)`
.replace(["NaT", "NaN", "nan"], np.nan)"
559e20c to
701e4f4
Compare
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 PR fixes deprecation warnings from matplotlib and pandas in the test suite by updating deprecated function calls to their modern equivalents.
- Replaces deprecated
matplotlib.pyplot.plot_date()withplt.plot()throughout the summaryplot module - Updates pandas
replace()usage to suppress future downcasting warnings with explicit options - Adds test infrastructure for matplotlib image comparison testing
Reviewed Changes
Copilot reviewed 7 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_summaryplot.py | Adds matplotlib image comparison test function and imports |
| tests/test_ofmvol2csv.py | Suppresses pandas date parsing warnings in test |
| src/subscript/summaryplot/summaryplot.py | Replaces deprecated matplotlib functions and standardizes pyplot imports |
| src/subscript/fmuobs/writers.py | Updates pandas replace() to use explicit downcasting options |
| pyproject.toml | Adds pytest-mpl dependency for matplotlib testing |
| .github/workflows/subscript.yml | Updates CI to use uv package manager and enable matplotlib tests |
| .github/workflows/codecov.yml | Updates CI to use uv package manager |
Comments suppressed due to low confidence (1)
src/subscript/summaryplot/summaryplot.py:490
- [nitpick] The format string parameter should be passed as
fmtkeyword argument for clarity when other keyword arguments are present. Consider changing line 485 tofmt=\"-\",to maintain consistency with matplotlib's API conventions.
plt.plot(
summaryfile.dates,
values,
"-",
color=cycledcolor,
label=sumlabel,
linewidth=1.5,
alpha=alpha,
)
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Not sure if it is an irrelevant question , but should we consider using webp instead of png for figures? Or maybe that is not possible for usage in tests? |
I suspect that |
Let's keep it as is then. |
rnyb
left a comment
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.
LGTM
Should fix the following warnings from the tests: