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

Consistently use dimension/coordinate names in VariableInfo.coordinates #413

Merged
merged 2 commits into from
Jan 10, 2020
Merged

Consistently use dimension/coordinate names in VariableInfo.coordinates #413

merged 2 commits into from
Jan 10, 2020

Conversation

bouweandela
Copy link
Member

@bouweandela bouweandela commented Dec 23, 2019 •
edited
Loading

Closes #412

Tasks

  • Create an issue to discuss what you are going to do, if you haven't done so already (and add the link at the bottom)
  • This pull request has a descriptive title that can be used in a changelog
  • Add unit tests
  • Public functions should have a numpy-style docstring so they appear properly in the API documentation. For all other functions a one line docstring is sufficient.
  • Circle/CI tests pass. Status can be seen below your pull request. If the tests are failing, click the link to find out why.
  • Codacy code quality checks pass. Status can be seen below your pull request. If there is an error, click the link to find out why. If you suspect Codacy may be wrong, please ask by commenting.
  • make a new pull request in the ESMValTool repository to fix the ERA-Interim CMORizer and add the link below

Related to ESMValGroup/ESMValTool#1338

@bouweandela bouweandela changed the title Use coordinate names instead of axis Consistently use dimension/coordinate names in VariableInfo.coordinates Dec 23, 2019
This was referenced Dec 23, 2019
Merged
Merged
@mattiarighi mattiarighi added enhancement New feature or request cmor Related to the CMOR standard labels Jan 3, 2020
@bouweandela bouweandela changed the base branch from development to master January 3, 2020 12:40
@bascrezee
Copy link
Contributor

Thanks @bouweandela. Testing this now in combination with ESMValGroup/ESMValTool#1338 Looking good so far.

bascrezee
bascrezee approved these changes Jan 10, 2020
View reviewed changes
Copy link
Contributor

@bascrezee bascrezee left a comment

Choose a reason for hiding this comment

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

Tested with ESMValGroup/ESMValTool#1338 and works fine now.

jvegreg
jvegreg approved these changes Jan 10, 2020
View reviewed changes
Copy link
Contributor

@jvegreg jvegreg left a comment

Choose a reason for hiding this comment

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

Thanks for catching this!

@mattiarighi
Copy link
Contributor

Any other recipe/cmorizer that could be affected?

@bouweandela
Copy link
Member Author

Yes, I'll make the associated pull request.

Peter9192 added a commit that referenced this pull request Jan 10, 2020
@Peter9192
Add note/todo on #413
db2a638
@bouweandela
Copy link
Member Author

It looks like only the ERA-Interim CMORizer was affected, I adjusted ESMValGroup/ESMValTool#1338 so it works both with and without this pull request.

@bouweandela bouweandela merged commit 1525bf0 into master Jan 10, 2020
@bouweandela bouweandela deleted the variable_info_coord branch January 10, 2020 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bascrezee bascrezee bascrezee approved these changes

@jvegreg jvegreg jvegreg approved these changes

Assignees
No one assigned
Labels
cmor Related to the CMOR standard enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Coordinates of VariableInfo inconsistent
4 participants
@bouweandela @bascrezee @mattiarighi @jvegreg