-
Notifications
You must be signed in to change notification settings - Fork 33
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.
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
[1pt] PR: Convert CatFIM pipeline to open source #292
Conversation
Hey Brian, here are some things I noticed upon initial review:
|
I confirmed that the script ran all the way through. Nice! I have some feedback regarding the fields that I did not mention in previous comments:
The values for the fields listed above can be derived from the existing fields that are available post-join (e.g. |
Almost all of these are related to the input table I am joining to the spatial layer. Seems like those changes should be made there and not in this workflow. I'll get with @TrevorGrout-NOAA about resolving these. |
I think the fields in the list can only be created after the join because of the way the join table and the CatFIM library are formatted. We can chat more about it on a call though. But yes, the name of the |
Should be good to go. Trevor said he would deal with rounding in his feature branch. |
I ran it again and everything looks great, although we need to add |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving. The WFO
attribute will be represented after it is available in the necessary join table.
Convert CatFIM pipeline to open source to resolve issue #279
Changes
VIZ_PROJECTION
toshared_variables.py
inundation.py
util
folders undertools
directory