-
Notifications
You must be signed in to change notification settings - Fork 0
Supporting netCDF conventions test changes. #1
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
Supporting netCDF conventions test changes. #1
Conversation
|
@morwenna-01 You need to turn on travis-ci for your iris repo. Do you know how to do this? Log in to https://travis-ci.org/ and tick your repo so that travis-ci registers it's webhooks to run on PRs pushed against your repo. Any problems, then just let me know. |
lib/iris/fileformats/netcdf.py
Outdated
| warnings.warn(msg) | ||
| sman.update_global_attributes(Conventions=cube.attributes.get('Conventions')) | ||
| else: | ||
| sman.update_global_attributes(Conventions=conventions) |
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.
@morwenna-01 You seem to have backed out my change here. We need to move your override to here otherwise the preceding patching breaks. Plus the usage of has_key is not Python3 friendly.
|
|
||
| # Prioritise cube attributes conventions over the default. | ||
| if 'Conventions' in cube.attributes: | ||
| conventions = cube.attributes['Conventions'] |
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.
@morwenna-01 Note that, the Conventions override requires to be applied here, otherwise the following cf_patch_conventions patching will fail in our tests ...
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.
@bjlittle Sorry. As you can tell I'm new at this. Please tell me what I should do now.
It looks like I've got travis-ci working.
I'm guessing I should have done git pull or something.
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.
@morwenna-01 No worries ... I think that I can resolve this from my side. Let me try ...
978258d to
43cfdf4
Compare
|
@morwenna-01 Okay, so I done a rebase of my PR branch against your changed branch to resolve the problem. Let's see what travis-ci makes of the changes ... 😄 |
|
@morwenna-01 Whoop! Seems good 👍 |
|
@bjlittle Excellent. Now what? |
|
@morwenna-01 Okay, so if you merge this PR onto your branch, it will automatically update your PR #2457 on iris master. Which in turn, will result in the tests passing for your PR. That means it is in a state that can be merged. However, there is a bit of a debate going on ... as you might have noticed 😉 |
|
@morwenna-01 Nice one! 👍 |
@morwenna-01 Okay, here are the changes in support of your PR to get the tests passing.
If I've missed something, travis-ci will tell us and we can then rinse-and-repeat until done.