OSOceanAcoustics / echopype-examples

Echopype example notebooks
https://echopype-examples.readthedocs.io/
Apache License 2.0
9 stars 6 forks source link

Update notebooks to 0.6.3, index/front page, and README #27

Closed emiliom closed 1 year ago

emiliom commented 1 year ago

Update the 3 notebooks to run with 0.6.3. Includes:

Updated the front page with information about creating the conda environment and running it in binder (I copied and tweaked the binder info from the README); and added information to the README file about building the JupytertBook locally.

Note: In the hake notebook, I removed cell 2 after running the notebook, so the cell numbering now skips from cell 1 to cell 3. I had added an ep.verbose statement in cell 2, but I later realized that it was unnecessary b/c the behavior I was specifying was already the default.

review-notebook-app[bot] commented 1 year ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

github-actions[bot] commented 1 year ago

🚀 Deployed on https://deploy-preview-27--echopype-examples.netlify.app

emiliom commented 1 year ago

@leewujung How about we edit the title of the OOI notebook so it specifically says "OOI"? eg: "Watching a solar eclipse using an OOI moored echosounder". The hake notebook explicit mentions the hake survey in its title. I think OOI folks would appreciate the greater explicitness and visibility.

leewujung commented 1 year ago

Yes!

emiliom commented 1 year ago

How about we edit the title of the OOI notebook so it specifically says "OOI"? eg: "Watching a solar eclipse using an OOI moored echosounder"

Done

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

leewujung commented on 2022-11-04T17:32:10Z ----------------------------------------------------------------

Assemble a list of EchoData object from the converted Zarr files.

How about changing this to:

Assemble a list of EchoData object from the converted Zarr files and combine them into a single EchoData object using the combine_echodata function.

And of course the following sentence needs to be tweaked a bit.


review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

leewujung commented on 2022-11-04T17:32:11Z ----------------------------------------------------------------

Suggest changes to make it explicitly that it is the combined object:

The Provenance group of the combined EchoData object now contains some helpful information compiled from the source files.


review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

leewujung commented on 2022-11-04T17:34:57Z ----------------------------------------------------------------

Note that this step is possible only because there are no duplicated frequencies present.

Small change:

Note that this step is possible only when there are no duplicated frequencies present.


leewujung commented 1 year ago

Ok, thanks @emiliom , I've finally reviewed this. Everything looks good and I made some minor suggested changes. I think the main README to benefit from adding some subheadings, but in the spirit to getting this updated first, feel free to merge after those changes are done. I'll do a separate PR on the subheadings.

emiliom commented 1 year ago

Thanks for the review. I've pushed the changes you suggested; BTW, wouldn't it be nice if GitHub + ReviewNB allowed for direct suggestions to be made for notebook markdown cells, and for direct merging?!

Feel free to submit a PR for the README file any time :stuck_out_tongue_winking_eye:

Merging. Woo-hoo!