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

0624/polar quivers notebook #650

Merged
merged 51 commits into from
Nov 17, 2023
Merged

0624/polar quivers notebook #650

merged 51 commits into from
Nov 17, 2023

Conversation

b-barton
Copy link
Collaborator

@b-barton b-barton commented Nov 9, 2023

Pull request checklist

Please check if your PR fulfills the following requirements:

  • [x ] Tests for the changes have been added (for bug fixes / features)
  • [x ] Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • [x ] Build (./build.sh) was run locally and no errors reported. NB not sure about this requirement: GitActions test this
  • [x ] Lint (pylint .) has passed locally and any fixes were made for failures. NB not sure about this requirement: GitActions test this with black

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • [x ] Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • [x ] Other (please describe): Example notebooks

What is the current behavior?

Issue URL: #

What is the new behavior?

New example notebooks for the Arctic domain. This shows how to plot quivers and use the functions to make adjustments for NEMO curvilinear grid and cartopy polar projection. An additional notebook has been added for plotting the new example dataset (a cropped version of a global NEMO run) for the Arctic. This includes plotting contours over the arctic which are not straight forward. Additional plotting helper functions have been added to plot_util for making polar plots and for plotting contours on polar plots. Tests included for these.

Checklist

  • [x ] Code compiles correctly
  • Created tests which fail without the change (if possible)
  • [~ ] All tests passing (pip install . && pytest unit_testing/unit_test.py -s)
    Same as before there are two tests I'm unable to run on my machine "test_gridded_initialisation" and "test_tidegauge_methods" (Windows things). The rest passed. I have no reason to think test_gridded_initialisation or test_tidegauge_methods would fail. Git actions tests keeps failing but I'm not sure why.
  • Extended the README / documentation, if necessary
  • [x ] Added myself / the copyright holder to the AUTHORS file

Does this introduce a breaking change?

  • Yes
  • [x ] No

Other information

@b-barton b-barton requested review from jpolton and soutobias November 9, 2023 16:36
@jpolton jpolton self-assigned this Nov 9, 2023
@jpolton
Copy link
Collaborator

jpolton commented Nov 9, 2023

Hmmm. There is definitely a problem with installing cartopy on the GitAction runners. The problem looks like the containers are trying to install cartopy from source, which is hard. The internet recommends using conda to install cartopy. However, the runners use a pip install method that uses the requirements.txt file. Perhaps it would be easier if the runners copied the environment.yml approach with slip pip and conda install methods? Hmmm

@jpolton
Copy link
Collaborator

jpolton commented Nov 9, 2023

Previously we installed cartopy but not pyproj. The problem now might be it is hard to mix these two packages as cartopy has pyproj under the hood...

Maybe explicit use of pyproj can be avoided: https://clouds.eos.ubc.ca/~phil/courses/atsc301/html/cartopy_mapping_pyproj.html looks like it might be possible
Then we would revert back to previous build requirements

@soutobias
Copy link
Collaborator

soutobias commented Nov 14, 2023

@jpolton @b-barton , I've made code updates by replacing cartopy with pyproj. Additionally, I've incorporated changes in the code to address pylint-related issues. All tests are passing, except for the pylint test. However, I've already addressed the pylint issue in a separate branch, which will resolve this issue. I also updated the example files and now they have only 5 mb.

Copy link
Collaborator

@soutobias soutobias 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 for me

@jpolton jpolton merged commit 080526d into develop Nov 17, 2023
12 of 14 checks passed
@jpolton jpolton deleted the 0624/polar_quivers_notebook branch November 17, 2023 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants