Skip to content

Conversation

@clouds56
Copy link

Motivation

Before this PR, BasicSweeper.split_arguments would keep only last override for the same key.
in case a=1,2,3 a=10,20,30 it works well, but in case a={x:10} a={y:20} it would have different behavior between MULTI_RUN and RUN.

This PR introduce BasicSweeper.simplify_overrides that would try its best to remove duplicated key override.

  1. it would automatically test if key is a group
  2. for group it would keep last override (that is OverrideType.CHANGE)
  3. for non-group, it would check if the value is dict or primitive (str, int, bool, float, list)
  4. dict and primitive is exclusive, only one will wins
  5. if value type is changed to dict, it would keep all dict till end
  6. if value type is changed to primitive, it would only keep last value

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

(How should this PR be tested? Do you require special setup to run the test or repro the fixed bug?)

Related Issues and PRs

fixes #3106
(Is this PR part of a group of changes? Link the other relevant PRs and Issues here. Use https://help.github.com/en/articles/closing-issues-using-keywords for help on GitHub syntax)

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 20, 2025
@clouds56
Copy link
Author

I have a question that, in order to detect if the key is group, we need config_loader, I'm not sure if we should use loader from Override or from BasicSweeper.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Basic sweeper would behavior wrong with value type dict

1 participant