NCAR / geocat-applications

GeoCAT Applications is a community resource inspired by the NCL Applications page.
https://ncar.github.io/geocat-applications/
Apache License 2.0
4 stars 5 forks source link

Trig functions and NCL receipt #40

Closed cyschneck closed 4 days ago

cyschneck commented 1 month ago

PR Summary

Trig functions

PR Checklist

General

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

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

github-actions[bot] commented 1 month ago

Meowdy! See your PR preview: 🔍 Git commit SHA: 8dbb1de48f88a876d382782fd53f5824a77f5fab ✅ Deployment Preview URL: https://NCAR.github.io/geocat-applications/_preview/40

jukent commented 1 month ago

Looks great. Thanks for the updates to the Contributing guide as well. It seems to be missing from the homepage though?

anissa111 commented 1 month ago

Just a few initial thoughts, I'll give this a more thorough look tomorrow 👍

anissa111 commented 1 month ago

Sorry, the review notebook app didn't let me leave a top-level review comment.

I'm thinking we should either refocus this notebook to be a receipt, or at least a trig focused ncl entry if there's anything that a receipt notebook produces that's significant enough to warrant adding an ncl notebook.

cyschneck commented 1 month ago

Ok, based on suggestions, I expanded this to cover the trig functions (sin, cos, tan, asin, acos, atan) since they are all very similar in execution. The notebook is now broken into:

kafitzgerald commented 1 month ago

Sorry, the review notebook app didn't let me leave a top-level review comment.

I'm thinking we should either refocus this notebook to be a receipt, or at least a trig focused ncl entry if there's anything that a receipt notebook produces that's significant enough to warrant adding an ncl notebook.

Oh gosh, I missed this. Sorry for the very duplicate review earlier.

cyschneck commented 4 weeks ago

I removed the changes made to the contributor's guide and put them into the relevant PR #52 to clean up this PR

cyschneck commented 1 week ago

Since the trig functions are using the same notebook, I don't think it will be able to populated by NCL Applications side bar since that is populated by separate notebooks