-
Notifications
You must be signed in to change notification settings - Fork 26
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
Update Jupyter examples #171
Conversation
This uses the following content from the first Markdown block: - The title is taken from the first heading (starting with '#') - The short description on the index page is the first non-heading paragraph (separated from other paragraphs following paragraphs by one or more blank lines)
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.
@speth ... thank you for this update. I just pulled the artifacts and the main page looks great! Just some minor comments from my side ...
One thing I noticed is that one of the newer examples isn't added yet: ck2yaml_demo
Beyond, I think that locations of YAML files could also be pointed out here, i.e.
If you have Cantera and the Jupyter Notebook server installed on your local machine, simply download any Jupyter notebook by clicking the "Source" link at the top of each example page and you should be able to run it.
[As an aside, if I follow these instructions on Firefox/Fedora, I get to some cryptic looking output for the artifact when I click on 'source', although I didn't experience this on the actual website.]
Spot-checking, the data files are displayed correctly for lithium_ion_battery.ipynb
, but are missing for equations_of_state.ipynb
. On that note, the latter notebook also has some problems with LaTeX rendering that I didn't notice elsewhere.
On interactive_path_diagram.ipynb
, the rendering of some of the headers is not pretty (likely the wrong header level).
Some of the issues I am pointing out are obviously not necessarily website related, but should be fixed by a minor update on cantera-jupyter
.
I'm happy to add these to Cantera/cantera-jupyter#34 |
I fixed the missing One case that I did not resolve is the input files for the I also did not see a rendering problem with |
Thank you! Example was merged very recently (after creation of this PR!), although I wrote it quite some time (2 years!) ago - Cantera/cantera-jupyter#27. Honestly didn't think about it until it came up here ...
I think that fell between the cracks - changes date back way before Cantera/cantera-jupyter#33, although both PR's were merged the same day. I am hesitant to fix this now as it will mess with Cantera/cantera-jupyter#34. Imho, there should be some distinction as CK input is a separate category - I think it's fine to leave as is?
I checked again today, and it may have something to do with my local OS. When viewing the downloaded artifacts yesterday, I had stray |
"Source" link. These data files should either be placed in a subdirectory named "data" | ||
one level up from the directory containing the notebook file, or the notebook can be | ||
modified to point to the location of these data files on your computer. |
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.
On second thought: would it make sense to just tell users to clone the cantera-jupyter
repo?
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.
No, I don't think we want to put the barrier of using Git between our users and some of our most readily accessible examples.
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.
Ok. In this case I’m ok with merging …
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.
Well, on second thought, I don't think it hurts to mention the option of cloning the repo as an alternative.
Sorry, I should have caught that at review.
Will do. |
Improve descriptions given in the index of Jupyter examples (in combination with Updates for Cantera 2.6 cantera-jupyter#33). For example, this:
becomes:
Automatically link to any input files used in each example that come from the
data
directory (also reorganized by Updates for Cantera 2.6 cantera-jupyter#33). For example: