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 stash mapping changes v2.0.0 -> v2.1.0 to changelog #3094

Merged
merged 4 commits into from
Oct 15, 2018

Conversation

tv3141
Copy link
Contributor

@tv3141 tv3141 commented Jul 23, 2018

Closes #2414.

I included only name changes, as referring to a cube by name is common.

I omitted unit changes and whether a STASH is translated into a standard name or a long_name.

@tv3141 tv3141 closed this Jul 24, 2018
@tv3141 tv3141 reopened this Jul 24, 2018
@DPeterK DPeterK self-assigned this Oct 3, 2018
@DPeterK
Copy link
Member

DPeterK commented Oct 3, 2018

Hi @tv3141, thanks for making this proposal to increase the detail in the Iris whatsnew. I do, however, have significant reservations about the change in its current form. The Iris whatsnew should detail changes made in Iris, but my concern is that the additions you're proposing here are not changes that have been made in Iris, merely changes that Iris inherits from metarelate and presents to users as Iris behaviour is affected by metarelate changes.

By means of an analogy, the change you're proposing here is like reproducing part of the netCDF4-python library's changelist because it impacts on how Iris behaves. Such a reproduction is unprecedented, and "not the done thing", beyond, perhaps, noting that Iris now uses a new version of netCDF4-python by default, which (minus the metarelate release version) is effectively what's happening here too.

To preempt a perhaps obvious issue with my argument, I'm aware that a change has been made to Iris in the um_cf_map file. I'm well aware that this is the case, but even then, the file in question makes it clear that the contents of this file are auto-generated from changed metarelate content. As such, this file is arguably not a part of Iris beyond needing to be included for the translations it provides.

Having said all that, I very much support the intention here if not the implementation, for the reasons detailed above. I think it's valuable for Iris users to be able to easily see the metadata translation changes that may impact them on release of a new Iris version with metadata translation updates. My proposal for doing this would be a single item in the whatsnew that looks something like the following:

## New Features
* ...
* This release of Iris contains a number of updated metadata translations. 
  See [this changelist](https://github.com/SciTools/iris/commit/69597eb3d8501ff16ee3d56aef1f7b8f1c2bb316#diff-1680206bdc5cfaa83e14428f5ba0f848) for further information.
* ...

So instead of laying out the changes in the whatsnew, we instead provide a link to the changes that have been made to metadata translations (either within Iris or directly from metarelate, I guess).

@corinnebosley
Copy link
Member

@bjlittle This WhatsNew entry is outdated now (since we have just released iris 2.2); is there still value in adding this entry retrospectively?

replaced list with link of stash mappings
@pp-mo
Copy link
Member

pp-mo commented Oct 15, 2018

is there still value in adding this entry retrospectively?

I'm with @dkillick on this : I don't think we should start doing this.

Unfortunately I think the key problem is the lack of versioning in https://github.com/metarelate/metOcean.
Ideally, we should be able to simply say "Updated new UM file translations from metarelate/metOcean from vX.YY to vX.ZZ", and point to some whatsnew-like summary of changes over a period there.
But as it is, metarelate/metOcean has no versioning, no releases and no changenotes :-(

@pp-mo
Copy link
Member

pp-mo commented Oct 15, 2018

metarelate/metOcean has no versioning

We can however just reference #3043, which shows the resulting changes in um_cf_map.py.

@corinnebosley corinnebosley merged commit 746b9b7 into SciTools:master Oct 15, 2018
@corinnebosley
Copy link
Member

Boom! Thanks @tv3141, your time and effort is much appreciated.

@pp-mo
Copy link
Member

pp-mo commented Oct 16, 2018

Oh yes, that is much nicer ! 💐

@DPeterK
Copy link
Member

DPeterK commented Oct 22, 2018

@tv3141 nice! Thanks for submitting this whatsnew update, and thanks @corinnebosley for the merge 👍

@SciTools SciTools deleted a comment from DPeterK Oct 23, 2018
@QuLogic QuLogic added this to the v2.3.0 milestone Oct 23, 2018
@tv3141 tv3141 deleted the amend_changelog branch November 8, 2018 12:58
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.

5 participants