Remove configure_callbacks #1647
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Unsure why pytest self cancelled |
|
OK some args may lead nowhere after this change, e.g. |
|
Let's wait for feedback from Lightning-AI/pytorch-lightning#18784 |
This has been happening to all of our tests recently and I have no idea why. Anyone who figures this out is my hero. |
|
Lightning-AI/pytorch-lightning#18784 proposed one possible workaround that will allow us to define defaults but also allow the user to override them. But I'm not sure if the added complexity is worth it. Thoughts? |
|
It does sound like a lot of extra complexity to take on and manage - I still think leaving it to the users to set their own callbacks whilst providing a few examples of best practice is the way to go |
adamjstewart
approved these changes
Nov 6, 2023
nilsleh
pushed a commit
that referenced
this pull request
Nov 6, 2023
* Remove configure_callbacks * Remove unused imports
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The use of configure_callbacks was blocking any use of
callbacksand caused #1641This is now removed. This will require the use to configure these callbacks but this is already documented here
Fixes #1640
Fixes #1641
Fixes #1642
Fixes #1645