deleted attrs in netcdf files #216
Conversation
micahkim23
changed the title
micahkim23
changed the title
closes #214
micahkim23
changed the title
closes #214
There was a problem hiding this comment.
@micahkim23 I downloaded the files and had a look offline; this looks good to me, thanks! I will let @spencerahill have the final word.
|
Thanks for checking @spencerkclark -- I believe you that they're good. @micahkim23, FYI for a PR like this that is in reference to a specific issue (#214 in this case), it's good practice to write "Closes #", a.k.a. in this case "Closes #214", as the first line of the PR description. That provides context to others who come across it, and also Github then automatically closes the Issue once the PR gets merged. Also, typically we add a note to the What's New section of our docs whenever we merge in something new. This particular one is small enough that we would normally skip it, but I want your handle on there! So please add a note under the v0.2.1 section, under a new subheading titled "Enhancements". Just emulate the other items in that page for the formatting/style. Thanks! |
|
@spencerahill I added my changes in the enhancement section of What's New. |
docs/whats-new.rst
Outdated
| Enhancements | ||
| ~~~~~~~~~~~~ | ||
|
|
||
| - Remove ``xarray.Dataset`` attributes from example netcdf files. |
There was a problem hiding this comment.
Strictly speaking, these are attributes of the netCDF files themselves, in addition to the xr.Dataset representations of the files. So replace "xarray.Dataset" attributes with "potentially confusing".
|
@micahkim23 one last nit, then I'll merge. |
|
@spencerahill sweet, made the fix. |
|
In it goes! Thanks again, and congrats on your first merged PR 🎉 Normally we would only merge if all of the continuous integration (CI) tests passed. But the one failure in Travis CI appears to be unrelated, likely a temporary system hangup. So I think we can safely ignore it. |
Closes #214