add fetchables for showcase notebook inputs #437
Conversation
dwhswenson
changed the title
fetch subcommand for showcase notebook
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #437 +/- ##
=======================================
Coverage 91.13% 91.14%
=======================================
Files 106 106
Lines 6241 6244 +3
=======================================
+ Hits 5688 5691 +3
Misses 553 553
☔ View full report in Codecov by Sentry. |
Co-authored-by: David W.H. Swenson <david.swenson@omsf.io>
mikemhenry
changed the title
fetch subcommand for showcase notebookfetch subcommand for showcase notebook inputs
mikemhenry
changed the title
fetch subcommand for showcase notebook inputs
|
@dwhswenson ready for review 🎉 |
|
Also once this gets merged in, I will need to cut a new release + make a new installer for our colab installer |
|
|
||
| from openfecli.fetching import URLFetcher, PkgResourceFetcher | ||
|
|
||
| _EXAMPLE_NB_BASE = ("https://raw.githubusercontent.com/" |
There was a problem hiding this comment.
If we change the url here to be against a tag/commit then we can't accidentally break these. If we are patching the files, it makes more sense to be intentional about updating this url to reflect that.
There was a problem hiding this comment.
@richardjgowers: Is the intent for try.openfree.energy to only update on releases? Like, a typo fix in notebook will only be reflected on a release of openfe?
If we do that, I'd recommend setting up some automation to check that that this is the most up-to-date. "Be intentional about updating the URL" sounds to me like "we're going to forget to update the URL."
There was a problem hiding this comment.
With the installer hard-coded + the URL using a tagged release, then we are pretty set in keeping try.openfree.energy stable. Even if we push a new release and have API changes, the notebook will still be using the "old" installer, so it will stay stable and just have to remember to update it.
There was a problem hiding this comment.
Yes I think try. should be off releases not HEAD
|
|
||
| from openfecli.fetching import URLFetcher, PkgResourceFetcher | ||
|
|
||
| _EXAMPLE_NB_BASE = ("https://raw.githubusercontent.com/" |
There was a problem hiding this comment.
@richardjgowers: Is the intent for try.openfree.energy to only update on releases? Like, a typo fix in notebook will only be reflected on a release of openfe?
If we do that, I'd recommend setting up some automation to check that that this is the most up-to-date. "Be intentional about updating the URL" sounds to me like "we're going to forget to update the URL."
openfecli/fetchables.py
Outdated
| ], | ||
| short_name="rbfe-showcase", | ||
| short_help="Inputs needed for the RBFE Showcase Notebook", | ||
| section="Tutorials", |
There was a problem hiding this comment.
Seems to me that, without the actual notebook, this is only intended for internal use. If so, change the section name (to anything, really, but I default to using "hidden") so that it doesn't show in help.
|
We should hold off merging this until people are happy with OpenFreeEnergy/ExampleNotebooks#69 (just in case I need to make changes to this PR) |
No description provided.