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

Delete equalise_cubes.py and add whatsnew #4496

Merged

Conversation

wjbenfold
Copy link
Contributor

🚀 Pull Request

Description

Remove iris.experimental.equalise_cube, closing #3528


Consult Iris pull request check list

@wjbenfold
Copy link
Contributor Author

Not sure where in what's new this belongs - is it Internal or a Deprecation (which I assumed were things we're deprecating now, not removing having previously deprecated them)

@wjbenfold wjbenfold linked an issue Jan 12, 2022 that may be closed by this pull request
@lbdreyer
Copy link
Member

I'm tempted to say it may make more sense for the whats new to be added to deprecations. We state that something will be deprecated, and then follow this up by confirming it has actually been removed, and no longer available. As a user, if I wanted to check when it was actually removed, my first port of call would be to look under the "deprecations" section as it is related to that.
I wouldn't think a user should necessarily have to concern themself with what goes in "internal" so they may not look there

@lbdreyer lbdreyer self-assigned this Jan 12, 2022
@lbdreyer
Copy link
Member

Don't think I made myself so clear in the lat comment, so to follow up: I think removing code is part of the deprecation process so the what's new should live under the deprecations heading

Copy link
Member

@rcomer rcomer left a comment

Choose a reason for hiding this comment

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

I agree it makes sense to have the whatsnew in internal. The user will see no functional change (other than a different error message if they try to import this module).

Edit: x-post with @lbdreyer and will defer to her!

@lbdreyer
Copy link
Member

Edit: x-post with @lbdreyer and will defer to her!

Whoops! danger of commenting at the same time! But I suppose this proves there's no correct answer; both have valid reasons.

Another point I wanted to mention though: I think this is a bit of a strange deprecation, in that we changed the code of the old function. Normally I would expect, when using the function, to get a deprecation warning, but to still be able to use it, until its eventual removal. In that example, I would say it definitely belongs in deprecations

@lbdreyer lbdreyer merged commit 9b64c94 into SciTools:main Jan 13, 2022
@lbdreyer
Copy link
Member

Thanks @wjbenfold !

@wjbenfold wjbenfold deleted the wjbenfold-remove-experimental-equalise_cubes branch January 31, 2022 09:35
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.

Remove experimental equalise_cubes
3 participants