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

Add support for string coordinates #657

Merged
merged 1 commit into from
Jul 1, 2020
Merged

Conversation

jvegreg
Copy link
Contributor

@jvegreg jvegreg commented May 28, 2020

To solve the issue raised by @ledm in ESMValGroup/ESMValTool#1621

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.
  • If writing a new/modified preprocessor function, please update the documentation
  • 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.
  • Please use yamllint to check that your YAML files do not contain mistakes
  • If you make backward incompatible changes to the recipe format, make a new pull request in the ESMValTool repository and add the link below

If you need help with any of the tasks above, please do not hesitate to ask by commenting in the issue or pull request.


Copy link
Contributor

@valeriupredoi valeriupredoi 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, cheers @jvegasbsc

@valeriupredoi
Copy link
Contributor

reran the test container that failed with a segfault coz of bleh ye olde hdf5

Copy link
Contributor

@mattiarighi mattiarighi left a comment

Choose a reason for hiding this comment

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

Looks fine to me, but @ledm should confirm whether this solves his issue.

@jvegreg jvegreg added this to the v2.0.0 milestone Jun 8, 2020
@sloosvel
Copy link
Contributor

sloosvel commented Jun 9, 2020

I tried this with data that contains string coordinates and it worked, thanks @jvegasbsc

@bouweandela bouweandela added bug Something isn't working cmor Related to the CMOR standard labels Jun 15, 2020
@bouweandela bouweandela merged commit 3f5340a into master Jul 1, 2020
@bouweandela bouweandela deleted the fix_check_regions branch July 1, 2020 09:29
@bouweandela
Copy link
Member

Thanks everyone! Merging this now, since today is the deadline for being included in the first version 2 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cmor Related to the CMOR standard
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants