-
Notifications
You must be signed in to change notification settings - Fork 14
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
Add planar horizontal viz #33
Conversation
264495f
to
e3593e8
Compare
@xylar You can review the scripts now if you like. I'm sure there are ways that we could make them better, but I think this is a reasonable level of effort given that these are temporary files for the hackathon. Let me know if you feel differently. I haven't added the documentation yet, but will. |
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.
Yep, this is fantastic! A few more tiny changes.
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.
A few more. Sorry these keep coming in in small batches. I just keep thinking I'm done and then playing with the code a bit more.
af73520
to
55233c6
Compare
@xylar I added some text to the developer's guide. I'm not sure whether/how to update the api though when the function is located in |
Great! The API needs to be added in an appropriate spot (maybe alphabetically somehow) here https://github.com/E3SM-Project/polaris/blob/main/docs/developers_guide/api.md |
I should go after |
487e4b2
to
d7d3788
Compare
@xylar Thanks! That did the trick |
@xylar Now that the docs are done it is ready to be merged, unless you have more feedback. |
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.
Looks great! Go ahead and merge with the green button!
@cbegeman, I missed this in my review but it looks like the 2 images you refer to in the docs didn't actually get added. Could you make a quick PR to add them when you get a chance? |
Never mind, I just added them in #38. I used the 2 images above. I hope that's what you had in mind. |
This PR ports the
plot_horiz_field
function fromcompass/ocean/tests/isomip_plus/viz/__init__.py
into a shared viz folder. There are relatively minor changes related to this being used outsideplot_horiz_series
and making the figure size match the domain aspect ratio. To demonstrate its functionality, it is added to theinitial_state
step of thebaroclinic_channel
test case.Two functions are also needed:
_remove_boundary_edges_from_mask
makes sure that the periodic edges are not included (those polygons would span from one edge of the periodic domain to the other and obscure the other polygons)_compute_cell_patches
is a direct port fromcompass/ocean/tests/isomip_plus/viz/__init__.py
_compute_edge_patches
is a new function analogous to_compute_cell_patches
Checklist
api.md
) has any new or modified class, method and/or functions listedTesting
comment in the PR documents testing used to verify the changes