Skip to content
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.

Sign up for GitHub

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

Jump to bottom

dewtemp_trh #101

Merged
merged 31 commits into from
Oct 8, 2024
Merged

dewtemp_trh #101

merged 31 commits into from
Oct 8, 2024

Conversation

cyschneck
Copy link
Contributor

@cyschneck cyschneck commented Aug 26, 2024 •
edited
Loading

PR Summary

Add Meteorology dewtemp geocat function to resolve NCL dewtemp_trh function

Covers:

Related Tickets & Documents

Closes #100

PR Checklist

General

  • PR includes a summary of changes
  • Link relevant issues, make one if none exist
  • If adding a new page, select which type:
    • NCL Entry
    • Receipt

@cyschneck cyschneck self-assigned this Aug 26, 2024
@review-notebook-app Review Notebook App
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 26, 2024 •
edited
Loading

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

@cyschneck cyschneck added ncl Related to ncl specific content ncl receipt Receipt files are small files with little to no narrative content ncl entry NCL entries are pages that explain specifically how to achieve something that was possible in NCL labels Aug 26, 2024
@cyschneck
Copy link
Contributor Author

Added pooch to the environment.yml to be able to load in geocat-datafiles in the receipt

@cyschneck cyschneck marked this pull request as ready for review August 30, 2024 18:39
@anissa111
Copy link
Member

anissa111 commented Aug 30, 2024 •
edited
Loading

Added pooch to the environment.yml to be able to load in geocat-datafiles in the receipt

Just a reminder, we're making environment changes in separate PRs now.

@cyschneck
Copy link
Contributor Author

Added pooch to the environment.yml to be able to load in geocat-datafiles in the receipt

Just a reminder, we're making environment changes in separate PRs now.

I'll move it over now, thanks for the reminder!

@cyschneck cyschneck mentioned this pull request Aug 30, 2024
Merged
2 tasks
@cyschneck
Copy link
Contributor Author

cyschneck commented Aug 30, 2024 •
edited
Loading

PR is up for pooch and geocat-comp at #109

@cyschneck
Copy link
Contributor Author

Is there a preference for documentation to show comp functions as either:

>>> from geocat.comp import dewtemp
>>> temp = 18 + 273.15
>>> rh = 46.5
>>> dewtemp(temp, rh) - 273.15
6.298141316024157

Or

>>> from geocat.comp.meteorology import dewtemp
>>> temp = 18 + 273.15
>>> rh = 46.5
>>> dewtemp(temp, rh) - 273.15
6.298141316024157

?

@kafitzgerald
Copy link
Collaborator

kafitzgerald commented Aug 30, 2024 •
edited
Loading

Is there a preference for documentation to show comp functions...

I think (Anissa should confirm) we usually do something like:

import geocat.comp as gc

gc.dewtemp()

This helps make it clear what package the function belongs to.

+1 for consistency on this though!

@anissa111
Copy link
Member

Hmm. Well. I'm going to message y'all over in slack.

@anissa111 anissa111 removed their request for review September 25, 2024 20:10
@cyschneck
Copy link
Contributor Author

I can leave it for now, but for curiosity sake, a relative humidity of 0, NCL returns:

ncl 1> print(dewtemp_trh(273, 0))
(0)	 0

kafitzgerald
kafitzgerald reviewed Sep 25, 2024
View reviewed changes
ncl/receipts/dewtemp_trh.ipynb Outdated Show resolved Hide resolved
ncl/ncl_index/ncl-index-table.csv Outdated Show resolved Hide resolved
ncl/ncl_entries/dewtemp.ipynb Outdated Show resolved Hide resolved
ncl/ncl_entries/dewtemp.ipynb Outdated Show resolved Hide resolved
ncl/ncl_entries/dewtemp.ipynb Outdated Show resolved Hide resolved
ncl/ncl_entries/dewtemp.ipynb Outdated Show resolved Hide resolved
@cyschneck cyschneck requested review from kafitzgerald, anissa111 and jukent and removed request for anissa111 and jukent September 26, 2024 19:39
jukent
jukent reviewed Sep 26, 2024
View reviewed changes
ncl/ncl_index/ncl-index-table.csv
@@ -36,3 +36,4 @@ NCL Function,Description,Python Equivalent,Notes
`sqrt <https://www.ncl.ucar.edu/Document/Functions/Built-in/sqrt.shtml>`__,"Computes the square root of its input","``math.sqrt()`` or ``numpy.sqrt()``",`example notebook <../ncl_entries/general_applied_math.ipynb#sqrt>`__
`sign_matlab <https://www.ncl.ucar.edu/Document/Functions/Contributed/sign_matlab.shtml>`__,"Mimic the behavior of Matlab's sign function","``numpy.sign()``",`example notebook <../ncl_entries/general_applied_math.ipynb#sign-matlab>`__
`round <https://www.ncl.ucar.edu/Document/Functions/Built-in/round.shtml>`__,"Rounds a float or double variable to the nearest whole number","``round()`` or ``numpy.round()``",`example notebook <../ncl_entries/general_applied_math.ipynb#decimalplaces-round>`__
`dewtemp_trh <https://www.ncl.ucar.edu/Document/Functions/Built-in/dewtemp_trh.shtml>`__,"Calculates the dew point temperature given temperature and relative humidity","``geocat.comp.dewtemp()``",`example notebook <../ncl_entries/dewtemp.ipynb>`__
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this point to the NCL receipt notebook?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually none of them do, so I'm pretty sure this is correct as is. Are NCL receipts only visible from the GitHub repository (for those really LOOKING for them)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can get to the receipts from the website, though it's not obvious.

If you scroll down to the end of the NCL index, there's a link to the receipts page

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh I think I used to see it was there when the table was shorter, but now that it's so long we might want a better solution (of course not in this PR).

@cyschneck cyschneck requested a review from jukent October 7, 2024 19:17
jukent
jukent approved these changes Oct 8, 2024
View reviewed changes
Copy link
Collaborator

@jukent jukent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @cyschneck !

@cyschneck cyschneck merged commit e46046b into NCAR:main Oct 8, 2024
2 checks passed
@cyschneck cyschneck deleted the dewtemp_100 branch October 8, 2024 18:47
github-actions bot pushed a commit that referenced this pull request Oct 8, 2024
@cyschneck @github-actions
Delete preview for pull request #101
f32f0e3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jukent jukent jukent approved these changes

@kafitzgerald kafitzgerald kafitzgerald approved these changes

@anissa111 anissa111 Awaiting requested review from anissa111

Assignees

@cyschneck cyschneck

Labels
ncl entry NCL entries are pages that explain specifically how to achieve something that was possible in NCL ncl receipt Receipt files are small files with little to no narrative content ncl Related to ncl specific content
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[GA-1513] Meteorology - dewtemp
4 participants
@cyschneck @anissa111 @kafitzgerald @jukent