Proposal: set default value of OptionalParameter to None#3411
Merged
dlstadther merged 2 commits intospotify:masterfrom Apr 4, 2026
Merged
Proposal: set default value of OptionalParameter to None#3411dlstadther merged 2 commits intospotify:masterfrom
dlstadther merged 2 commits intospotify:masterfrom
Conversation
Contributor
Author
|
@dlstadther Sorry. Could you review this? |
dlstadther
previously approved these changes
Apr 3, 2026
Collaborator
dlstadther
left a comment
There was a problem hiding this comment.
I'm good here. Are there any test cases that need to be added/updated?
Contributor
Author
|
@dlstadther Thank you for the review! I added a test to verify that a Task with OptionalParameter() (no explicit default) can be instantiated without a value and defaults to None. |
dlstadther
approved these changes
Apr 4, 2026
Collaborator
dlstadther
left a comment
There was a problem hiding this comment.
LGTM. Thanks for adding a test case!
This file contains hidden or 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
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.
Description
Currently, when using
OptionalParameter(and its variants like OptionalStrParameter), users must explicitly setdefault=None. If omitted, Luigi throws aMissingParameterException.Since the prefix
"Optional"strongly implies that the parameter can be omitted and will default to a null state, the current behavior is slightly counter-intuitive. This PR proposes changing the default value of optional parameters from_no_valuetoNone.expected behavior
actual behavior
Motivation and Context
To improve the developer experience by aligning the behavior of OptionalParameter with standard Python conventions and user expectations. This change reduces boilerplate code (removing the need to write default=None everywhere) and prevents unexpected errors when an optional parameter is simply omitted.
Have you tested this? If so, how?
Verified that the provided reproduction code now runs successfully and outputs as expected.