Clean-ups around task-mapping code #26879
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
uranusjr
force-pushed
the
simplify-expand-mapped-literals
branch
2 times, most recently
from
10eed9c to
abe3550
Compare
There should be no change in functionality. A few things are involved: 1. The expand_mapped_literals closure is declaring a 'sequence' argument that is always None. Remove it. 2. The run_time_mapped_ti_count method is never used in isolation, but combined with parse_time_mapped_ti_count. We should just combine the calls -- actually, the run-time variant already encompasses the parse-time variant, so the latter call can simply be removed. 3. The TaskInstance import in _revise_mapped_task_indexes is redundant (the class is already imported globally) and is removed. 4. Various typing fixups.
uranusjr
force-pushed
the
simplify-expand-mapped-literals
branch
from
abe3550 to
38707b4
Compare
dstandish
approved these changes
Oct 11, 2022
ephraimbuddy
added
the
type:misc/internal
ephraimbuddy
pushed a commit
that referenced
this pull request
Oct 18, 2022
(cherry picked from commit a2d8724)
37 tasks
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.
I found several places where the code is awkward (although technically correct) and decided to change them. There should be no change in functionality. A few things are involved:
expand_mapped_literalsclosure is declaring asequenceargument that is alwaysNone. Remove it.run_time_mapped_ti_countmethod is never used in isolation, but combined withparse_time_mapped_ti_count. We should just combine the calls—actually, the run-time variant already covers the parse-time variant, so the latter call can simply be removed.TaskInstanceimport in_revise_mapped_task_indexesis redundant (the class is already imported globally) and is removed.