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

Do not write an empty "coordinates" attribute to a netCDF file #5121

Merged
merged 4 commits into from
Apr 7, 2021
Merged

Do not write an empty "coordinates" attribute to a netCDF file #5121

merged 4 commits into from
Apr 7, 2021

Conversation

alexamici
Copy link
Collaborator

@alexamici alexamici commented Apr 6, 2021 •
edited
Loading

  • No issue, trivial cleanup
  • Tests modified to match

The behaviour is not too problematic as CF checkers don't complain about the empty string, but it is aesthetically annoying.

This happens more frequently after #2844 added the auxiliary variable to .coords.

@alexamici alexamici mentioned this pull request Apr 6, 2021
Closed
@alexamici alexamici requested review from keewis and max-sixty April 7, 2021 06:45
dcherian
dcherian approved these changes Apr 7, 2021
View reviewed changes
Copy link
Contributor

@dcherian dcherian left a comment

Choose a reason for hiding this comment

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

Thanks @alexamici

keewis
keewis approved these changes Apr 7, 2021
View reviewed changes
Copy link
Collaborator

@keewis keewis 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. We could change the comparison, but this doesn't really change the readability so it's your choice. I don't know much about the current behavior so I can't comment on changing the test vs. writing a new one.

@dcherian dcherian merged commit e569058 into pydata:master Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dcherian dcherian dcherian approved these changes

@keewis keewis keewis approved these changes

@max-sixty max-sixty Awaiting requested review from max-sixty

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@alexamici @dcherian @keewis