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

Deprecate legacy_custom_rules #1836

Merged
merged 2 commits into from
Nov 17, 2015
Merged

Deprecate legacy_custom_rules #1836

merged 2 commits into from
Nov 17, 2015

Conversation

rhattersley
Copy link
Member

  1. Deprecates use of legacy_custom_rules.
  2. Gets rid of related unused bits:
    • iris.fileformats.grib._load_rules
    • iris.fileformats.pp._load_rules
    • iris.fileformats.pp._ensure_load_rules_loaded

@rhattersley rhattersley added this to the v1.9 milestone Nov 12, 2015
@rhattersley
Copy link
Member Author

I've added this to the 1.9 milestone to make sure we get the deprecation out before 2.0. Does that seem reasonable?

@pp-mo
Copy link
Member

pp-mo commented Nov 12, 2015

@rhattersley I've added this to the 1.9 milestone ... Does that seem reasonable?

Absolutely.

Could we not do more, though?
We still have supported (but essentially unused) functions for handling text rules
i.e. pp.add_save_rules + pp.reset_save_rules.

If we deprecated all this stuff, I think we are then free to reimplement the pp save rules + remove all the text rules legacy ?
(caveat: I haven't looked at it all, there may be more uses I've forgotten about)

@rhattersley
Copy link
Member Author

Could we not do more, though?

Absolutely. 😉

But I'd prefer to chip away at it in multiple PRs.

If we deprecated all this stuff, I think we are then free to reimplement the pp save rules + remove all the text rules legacy ?

👍

NB. Just because we've deprecated something doesn't mean we have to remove in v2.0, so we're not committing ourselves to anything.

@pp-mo
Copy link
Member

pp-mo commented Nov 12, 2015

Just because we've deprecated something doesn't mean we have to remove in v2.0, so we're not committing ourselves to anything.

No, but we need to move sharp if we want to leave the option open.

@rhattersley
Copy link
Member Author

Indeed. Are you going to put up a PR for the deprecations you suggest then?

@pp-mo pp-mo mentioned this pull request Nov 13, 2015
@rhattersley rhattersley force-pushed the rules branch 2 times, most recently from ea7365e to baced99 Compare November 16, 2015 20:54
@rhattersley
Copy link
Member Author

@pp-mo - I've rebased to get rid of the #1837 problem (and snuck in some tests while I was at it!). All tests pass. 😄

pp-mo added a commit that referenced this pull request Nov 17, 2015
Deprecate legacy_custom_rules
@pp-mo pp-mo merged commit c64f238 into SciTools:master Nov 17, 2015
@pp-mo
Copy link
Member

pp-mo commented Nov 17, 2015

Looks good to me :-)

@rhattersley rhattersley deleted the rules branch November 18, 2015 10: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.

2 participants