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.

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

Rename Arctic transects tag and flip OSNAP sections #185

Merged

Conversation

milenaveneziani
Copy link
Collaborator

This does two things:

  1. renames the arctic_sections tag to arctic_transport_sections to make it consistent with standard_transport_sections, and also as a reminder that the associated geometric features are mostly used for integrated transport calculations;
  2. flips the OSNAP sections to follow the east-to-west direction that is necessary for the positive-northward transport convention in the Northern Hemisphere.

@milenaveneziani
Copy link
Collaborator Author

milenaveneziani commented Sep 22, 2022

@xylar: I added the script that I used for doing this. There are some small formatting changes (in the affected geojson features). Please take a look and see if they are ok. Thanks.

@stephenprice

This comment was marked as off-topic.

Copy link
Collaborator

@xylar xylar left a comment

Choose a reason for hiding this comment

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

Looks good to me. I think we should test it in MPAS-Analysis, though, before we merge.

@xylar
Copy link
Collaborator

xylar commented Sep 25, 2022

@milenaveneziani, we probably need to add an aggregator like this one:
https://github.com/MPAS-Dev/geometric_features/blob/master/geometric_features/aggregation/ocean/transport_transects.py
and a section like this one:
https://github.com/MPAS-Dev/geometric_features/blob/master/geometric_features/aggregation/__init__.py#L59-L61
for the Arctic Transport Transects.

Do you want to include that in this PR, too?

Copy link
Collaborator

@xylar xylar left a comment

Choose a reason for hiding this comment

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

This looks good to me. I'd like to make sure it works well with MPAS-Dev/MPAS-Analysis#910 at least and maybe other MPAS-Analysis branches before we merge.

@xylar xylar merged commit af1eb70 into MPAS-Dev:master Dec 8, 2022
@xylar xylar removed the request for review from alicebarthel December 8, 2022 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants