Implement discharge capacity as an optional x-axis in QuickPlot
#4775
Conversation
medha-14
requested review from
martinjrobins,
rtimms,
Saransh-cpp,
valentinsulzer,
kratman and
agriyakhetarpal
as code owners
|
I have implemented the necessary changes for handling 0D variables. Once this is confirmed to be working correctly, I plan to work on handling 1D and 2D variables. Additionally, I have made changes in the |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #4775 +/- ##
===========================================
- Coverage 98.69% 98.62% -0.08%
===========================================
Files 303 303
Lines 23226 23252 +26
===========================================
+ Hits 22923 22932 +9
- Misses 303 320 +17 ☔ View full report in Codecov by Sentry. |
|
Hello @agriyakhetarpal could please give a review on this? :) |
There was a problem hiding this comment.
Thanks, this is a good start.
Variables should always be evaluated with time, not discharge capacity.
Also, the implementation would be simpler if you just set self.x_axis, self.x_min and self.x_max once and used those everywhere, instead of having the if/else for time and discharge capacity in so many places
| if self.x_axis == "Discharge capacity [A.h]": | ||
| var_min = np.min( | ||
| [ | ||
| ax_min(var(self.dc_values[i], **spatial_vars)) |
There was a problem hiding this comment.
Whenever a variable is being evaluated, it should always be evaluated with time as the first argument, regardless of how the x-axis is defined. So this change is not necessary (also applies in a few other places)
| ) | ||
| var_max = np.max( | ||
| [ | ||
| ax_max(var(self.dc_values[i], **spatial_vars)) |
| full_dc = self.dc_values[i] | ||
| (self.plots[key][i][j],) = ax.plot( | ||
| full_dc, | ||
| variable(full_dc), |
Description
Fixes #1751
Type of change
Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.
Key checklist:
$ pre-commit run(or$ nox -s pre-commit) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)$ python -m pytest(or$ nox -s tests)$ python -m pytest --doctest-plus src(or$ nox -s doctests)You can run integration tests, unit tests, and doctests together at once, using
$ nox -s quick.Further checks: