Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Move from @cbook.deprecated to @_api.deprecated#18823

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

timhoffm
Copy link
Member

PR Summary

Follow-up to#18657, which moved the functions. This PR now starts to move their usages in our code base.

@timhoffmtimhoffm added this to thev3.4.0 milestoneOct 26, 2020
@QuLogic
Copy link
Member

Mostly looking okay, but you have many missing references.

@QuLogic
Copy link
Member

QuLogic commentedOct 27, 2020
edited
Loading

Actually, no not okay,_api does not expose any of the deprecation machinery directly like that.

@timhoffmtimhoffm marked this pull request as draftOctober 27, 2020 09:03
@QuLogicQuLogic mentioned this pull requestOct 29, 2020
5 tasks
@timhoffmtimhoffmforce-pushed theupdate-deprecation-api branch 2 times, most recently from51444ba tobe20463CompareOctober 29, 2020 22:08
@timhoffmtimhoffm marked this pull request as ready for reviewOctober 30, 2020 00:13
@brunobeltran
Copy link
Contributor

brunobeltran commentedNov 4, 2020
edited
Loading

Why not justfrom _api.deprecation import deprecated? Instead of just making_api the new catch-all name space for helper functions?

Don't really have a strong opinion here but I figure you've given it some thought so wanted to know.

@timhoffm
Copy link
MemberAuthor

timhoffm commentedNov 4, 2020
edited
Loading

I'm a bit paranoid by now concerning semi-public API. Importingdeprecated leaks the name into the module namespace. - I know that other imports do that as well, but we can prevent in this case quite easily.

Also, I don't want to import the specific functions (check_in_List, warn_deprecated, make_keyword_only, rename_parameter, ...) and remove the imports when they expire. That's a big back and forth adapting of the imports. One could even consider inlining the deprecation module into _api, but for now I'd leave it as a sub module.

Furthermore, I like the _api prefix indicating the context. For example it makes it immediately clear, that_api.check_in_list() is some sort of standard function within our framework and not only a local helper functioncheck_in_list().

brunobeltran reacted with thumbs up emoji

Copy link
Contributor

@brunobeltranbrunobeltran left a comment

Choose a reason for hiding this comment

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

I'm convinced by the argument that everything should live under_api. At that point, I would honestly prefer if everything was just inlined into_api directly (especially since I'm about to move one random function into_api.deprecation just to avoid circular imports (#18817)), but it doesn't seem like doing it this way for now fundamentally hurts our ability to do differently moving forward so...it'd be good to get this in.

@QuLogicQuLogic merged commit1c20824 intomatplotlib:masterNov 5, 2020
@timhoffmtimhoffm deleted the update-deprecation-api branchNovember 5, 2020 12:06
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@QuLogicQuLogicQuLogic approved these changes

@brunobeltranbrunobeltranbrunobeltran approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
v3.4.0
Development

Successfully merging this pull request may close these issues.

3 participants
@timhoffm@QuLogic@brunobeltran

[8]ページ先頭

©2009-2025 Movatter.jp