search

ioos / notebooks_demos

Notebook demonstrations and examples
https://ioos.github.io/notebooks_demos/
MIT License
19 stars 19 forks source link

Adding jupyter{book} for Demo Data Center #409

Closed lohithmunakala closed 3 years ago

lohithmunakala commented 3 years ago

There was a notebook issue and that has been resolved. This is with respect to the PR https://github.com/ioos/notebooks_demos/pull/408 and https://github.com/ioos/notebooks_demos/pull/394 . The updated branch without conflicts is mentioned here.

The view is the same as shown in #408.

@ocefpaf please let me know if this is fine.

review-notebook-app[bot] commented 3 years ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

lohithmunakala commented 3 years ago

@MathewBiddle @ocefpaf are these fails common because the others PR's seem to have the same issue?

ocefpaf commented 3 years ago

The PR is massive, 615 files changed! That is quite hard to deal with.

One way to reduce would be to reset the changes in the notebooks. Changes like this one: https://github.com/ioos/notebooks_demos/pull/409/files#diff-7c024449328887c11902b4b4b457ee5a50f32dc94de8fcc7422bb9c9810f9ca0

Can you do that? Also, please confirm upstream if we need to commit the _build directory. That would also reduce the noise level on the PR. Or maybe we can commit them in another PR?

lohithmunakala commented 3 years ago

Yes I will do that. Will look at reducing the redundancy of the PR

lohithmunakala commented 3 years ago

I have removed all the build files to keep the initial PR clean. Please check and let me know @ocefpaf.

ocefpaf commented 3 years ago

I have removed all the build files to keep the initial PR clean. Please check and let me know @ocefpaf.

This is great. We are down to 53 files :-) much easier to review.

Please take a look at my comments at your convenience.

lohithmunakala commented 3 years ago

I have removed all the build files to keep the initial PR clean. Please check and let me know @ocefpaf.

This is great. We are down to 53 files :-) much easier to review.

Please take a look at my comments at your convenience.

I have made quite some changes to the _config.yml file. Included the above changes and made it easier to understand and included all the default values too (that were previously not included).

lohithmunakala commented 3 years ago

@ocefpaf, there is a new format for the _toc.yml file in the latest update (0.11.1) for jupyter{book}. I have changed the _toc.yml according to that. https://github.com/ioos/notebooks_demos/pull/409/commits/d84e8699aab2ccb2c683a08e4d0d9367ce9c8766

ocefpaf commented 3 years ago

@lohithmunakala this may look like the "easy" part but I guess you saw that the devil is in the details! You did an awesome work and hopefully the next PRs will be more fun coding rather than infrastructure discussion. Congrats on your first GSoC PR merged!

lohithmunakala commented 3 years ago

@lohithmunakala this may look like the "easy" part but I guess you saw that the devil is in the details! You did an awesome work and hopefully the next PRs will be more fun coding rather than infrastructure discussion. Congrats on your first GSoC PR merged!

@ocefpaf, @MathewBiddle Thanks to you guys! You've been helpful all the way!