search

Cantera / cantera-website

Official Cantera Website
https://cantera.org
Other
15 stars 26 forks source link

Update Jupyter examples #171

Closed speth closed 2 years ago

speth commented 2 years ago
bryanwweber commented 2 years ago

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 https://github.com/Cantera/cantera-jupyter/pull/34

speth commented 2 years ago

I fixed the missing ck2yaml example (a new category should have been added when that example was added), added directions explaining that the data files also need to be downloaded, and fixed the cases where the data files weren't being detected.

One case that I did not resolve is the input files for the ck2yaml example, which breaks the pattern of using the data directory for all input files. I think those input files should be moved to conform to the pattern of always using the data directory, but that needs to be dealt with on the cantera-jupyter repository.

I also did not see a rendering problem with equations_of_state.ipynb - could you be more specific (not that I think that has anything to do with this PR).

ischoegl commented 2 years ago

I fixed the missing ck2yaml example (a new category should have been added when that example was added) [...]

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 ...

One case that I did not resolve is the input files for the ck2yaml example, which breaks the pattern of using the data directory for all input files. I think those input files should be moved to conform to the pattern of always using the data directory, but that needs to be dealt with on the cantera-jupyter repository.

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 also did not see a rendering problem with equations_of_state.ipynb - could you be more specific (not that I think that has anything to do with this PR).

I checked again today, and it may have something to do with my local OS. When viewing the downloaded artifacts yesterday, I had stray $'s in my output, but they're gone today.

bryanwweber commented 2 years ago

I fixed the missing ck2yaml example (a new category should have been added when that example was added),

Sorry, I should have caught that at review.

but that needs to be dealt with on the cantera-jupyter repository.

Will do.