-
Couldn't load subscription status.
- Fork 342
Fixes and additions for new MODFLOW USG Transport support #2617
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 @@
## develop #2617 +/- ##
===========================================
+ Coverage 55.5% 72.5% +17.0%
===========================================
Files 644 667 +23
Lines 124135 129018 +4883
===========================================
+ Hits 68947 93623 +24676
+ Misses 55188 35395 -19793
🚀 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.
Presumably @aymanalz had this but it accidentally didn't make it into #2575?
I think if this is exercised by an example notebook there is no need for a test. But we wouldn't turn one down by any means. And I see now the new notebooks are getting skipped in CI tests and omitted from the gallery in RTD, that's why this wasn't caught.
If you like, you can fix that by
- renaming the new notebooks something with "tutorial" or "example" in the name
- adding them to
examples.rstortutorials.rst
I'm also happy to do that separately, either way
|
I opened a nice box of pandora adding the notebooks to the tests :) I fixed some of the failing notebooks, but some of them still don't run. Maybe @aymanalz can have a look as well as he made the notebooks. Because there are clearly some things missing:
gages = [[-1, -37, 3]]
files = [f"{mf.name}.got"]
gage = ModflowGage(mf, numgage=1, gage_data=gages, files=files) |
|
Ah, ok. So the notebooks still need some work. Well, thanks for opening the box @martinvonk. I bumped this to the next milestone as 3.9.4 is imminent. |
Okay! We could also merge this and fix the other two notebooks later? With this PR at least some of the notebooks work. |
|
@martinvonk that sounds good to me but I'll let @christianlangevin make the call. In any case, you can skip the two failing notebooks for now? |
I broke the tutorial RTD page when trying to add the new mfusg transport tutorials in #2617. Fix it. Correctly ignore the two new notebooks needing attention. And remove examples.rst and tutorials.rst, they're generated when the rtd assets are built.
Fixes #2616.
Do you want a test for this? I could not find a test for the
get_angldegxmethod so I did not make one for this method either. But I could make one (for both) if you guys want.