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

Fix for CI - Keep using older natural_earth version for testing #212

Merged
merged 2 commits into from
Mar 17, 2022

Conversation

jbusecke
Copy link
Owner

@jbusecke jbusecke commented Mar 16, 2022

Natural Earth recently released a new version that changed some of the marine regions (e.g. #208), leading to incompatibility issues.

Here I am using some new regionmask features to make sure that the older version is used for testing.
This is an attempt to quickly fix the CI (which has failed for the last few weeks), but I am wondering if it would be worth in the long run, to upgrade to the latest NE version.

Related question: @mathause, is there a way to identify the NE version from a regions object?

Something similar to this:

basins = regionmask.defined_regions.natural_earth_v4_1_0.ocean_basins_50
assert basins.version == 'something'

?
That way I could check that in my module and give a more useful error message

TODO:

  • Mention this in the docs. Link to here

Natural Earth recently released a new version that changed some of the marine regions (e.g. #208), leading to incompatibility issues. 

Here I am using some new regionmask features to make sure that the older version is used for testing.
This is an attempt to quickly fix the CI (which has failed for the last few weeks), but I am wondering if it would be worth in the long run, to upgrade to the latest NE version.

Related question: @mathause, is there a way to identify the NE version from a regions object? 

Something similar to this:
```python
basins = regionmask.defined_regions.natural_earth_v4_1_0.ocean_basins_50
assert basins.version == 'something'
```
?
That way I could check that in my module and give a more useful errormessage

TODO:
- [ ] Mention this in the docs.
@jbusecke jbusecke changed the title Keep using older natural_earth version Fix for CI - Keep using older natural_earth version for testing Mar 16, 2022
jbusecke added a commit that referenced this pull request Mar 16, 2022
Additionally to the problem with regionmask (which I started working on in #212), there seems to be an issue with a test asserting that a UserWarning is raised.

I believe the issue here is that suddenly more than one warning is raised and my old implementation did only check the first warning. 
I have now changed this to a list comparison.
* Attempt to fix test checking for user warning

Additionally to the problem with regionmask (which I started working on in #212), there seems to be an issue with a test asserting that a UserWarning is raised.

I believe the issue here is that suddenly more than one warning is raised and my old implementation did only check the first warning. 
I have now changed this to a list comparison.

* Update test_drift_removal.py

* [skip-ci]
@jbusecke
Copy link
Owner Author

Ill merge this and add an issue to remind me to add this to the docs

@mathause
Copy link

Related question: @mathause, is there a way to identify the NE version from a regions object?

Something similar to this:

basins = regionmask.defined_regions.natural_earth_v4_1_0.ocean_basins_50
assert basins.version == 'something'

No unfortunately not - I didn't think of this but it's a good suggestion. version would only be relevant for natural earth regions at the moment, but I guess I can just default it to None.

@jbusecke
Copy link
Owner Author

That would be amazing to have. Happy to help with implementation if that is useful!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants