-
Notifications
You must be signed in to change notification settings - Fork 31
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
Add plotting module #425
Add plotting module #425
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #425 +/- ##
==========================================
- Coverage 99.02% 98.73% -0.29%
==========================================
Files 16 17 +1
Lines 2558 2698 +140
==========================================
+ Hits 2533 2664 +131
- Misses 25 34 +9 ☔ View full report in Codecov by Sentry. |
assert ( | ||
hasattr(PyscalList(), "pyscal_list") is True | ||
), "The PyscalList object should have a pyscal_list instance variable.\ | ||
This is accessed by the plotting module." |
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.
Can you also test if CLI is creating figure as it should be?
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.
I have added a number of tests for the plotting
module in a42fc88
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.
Could you please update the PR? Thank's a lot @richypitman
… module. Add plot output directory option.
…anges such as adding docstrings
@alifbe I think this is ready to be approved 😃 |
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.
Looks good to me and don't forget to squash 😄
Please consider my comment about CLI testing.
captured = capsys.readouterr() | ||
|
||
for plot in expected_plots: | ||
assert plot in captured.out |
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.
Nice.
I think more robust way is to check if the file exist in the tmpdir
But it's more of a suggestion.
Added a plotting module with functionality to request plots via the pyscal CLI. One plot is made for each system (oil-water/gas-water/oil-gas) and each SATNUM. For curves interpolated using SCALrecommendation, the interpolated curves are plotted. By default, requesting plots will create relative permeability plots. There is an option to request a capillary pressure plot. Figures are saved in the current working directory. Some basic tests have also been added.