-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Rename the TrainingTypePlugin
base to Strategy
#11120
Conversation
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
for more information, see https://pre-commit.ci
…nto refactor/strategy/rename-base
Co-authored-by: Rohit Gupta <[email protected]>
I marked this as breaking change as user who extend TrainingTypePlugin will need to update their class inheritance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a heavy breaking change on the import level itself for users who extend TrainingTypePlugin. Can’t we prevent it by introducing strategy.py
file in the PR?
@kaushikb11 But then we only delay the breaking changes. I think we could introduce a |
@kaushikb11 To prevent a huge PR, I would prefer to go step by step by renaming parts individually. Once we move the files we can still add a simple reroute/deprecation path for mapping the training_type module to strategy |
What does this PR do?
Renames the base class to
Strategy
.The next PRs will cover all related usages:
TrainingTypePluginsRegistry
toStrategyRegistry
#11122Part of #10549
Before submitting
PR review
Anyone in the community is free to review the PR once the tests have passed.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:
Did you have fun?
I made sure I had fun coding 🙃
Part of #1 (it's a lie, this is just here to avoid noisy GitHub bot)
cc @Borda @justusschock @awaelchli @akihironitta