Implement `discharge capacity` as an optional x-axis in `QuickPlot`
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.
- [x] New feature (non-breaking change which adds functionality)
- [ ] Optimization (back-end change that speeds up the code)
- [ ] Bug fix (non-breaking change which fixes an issue)
Key checklist:
- [ ] No style issues:
$ 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) - [ ] All tests pass:
$ python -m pytest(or$ nox -s tests) - [ ] The documentation builds:
$ 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:
- [ ] Code is commented, particularly in hard-to-understand areas
- [ ] Tests added that prove fix is effective or that feature works
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 dynamic plot, though I am unsure if they were necessary, so I would appreciate it if you could review them as well. Could you please suggest the next steps for moving forward?
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 98.58%. Comparing base (
72df274) to head (cfcd43f).
Additional details and impacted files
@@ Coverage Diff @@
## develop #4775 +/- ##
========================================
Coverage 98.57% 98.58%
========================================
Files 304 304
Lines 23656 23675 +19
========================================
+ Hits 23320 23339 +19
Misses 336 336
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
Hello @agriyakhetarpal could please give a review on this? :)
Also, the implementation would be simpler if you just set
self.x_axis,self.x_minandself.x_maxonce and used those everywhere, instead of having the if/else for time and discharge capacity in so many places
I have made the suggested changes, I would like a review on if the plot functionality is working correctly or do I have to make some other changes.
I think the change looks good, could you please add a test or two? I expect the coverage to go down as you have a few branches in the code now (i.e., one for time, and one for discharge capacity).
Do I also have to make similar changes for 1D and 2D variables? or are they simply not applicable in those cases?
For now I have added tests for the function , let me know if any other changes are to be made
Do I also have to make similar changes for 1D and 2D variables? or are they simply not applicable in those cases?
Also suggest how to move forward with this?
I have made the suggested changes, if anyone could review this one :)
It looks good to me, just three missing lines in the coverage. Thanks for your work!
I have added tests for the missing lines, are there any other improvements I need to make ?