-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Recursive Item Mapping for Nested Lists in Compose #8187
base: dev
Are you sure you want to change the base?
Conversation
Signed-off-by: YunLiu <[email protected]>
Signed-off-by: YunLiu <[email protected]>
Hi @KumoLiu and thanks for the fix! Is this branch ready for testing? |
Signed-off-by: YunLiu <[email protected]>
Signed-off-by: YunLiu <[email protected]>
Signed-off-by: YunLiu <[email protected]>
Signed-off-by: YunLiu <[email protected]>
@atbenmurray please review this as well as we just discussed. |
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.
Looking at it, map_items=True has always acted as map_items=1, so at least we aren't changing the behaviour of using bool and now the int indicates the maximum amount of nesting that can be expected. This would also be the case if we use a multi sample transform after a multi sample transform. Its a bit opaque semantically that we do this and might benefit from additional commenting.
Co-authored-by: Ben Murray <[email protected]> Signed-off-by: YunLiu <[email protected]>
Signed-off-by: YunLiu <[email protected]>
Signed-off-by: YunLiu <[email protected]>
Hi @atbenmurray, thanks for the suggestion, I enhance the docstring in the latest commit. Please help take a look whether it looks clearer to you. |
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.
minor tweak required due to a typo in my earlier suggestion
Co-authored-by: Ben Murray <[email protected]> Signed-off-by: YunLiu <[email protected]>
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.
Happy with these comments!
Fixes #8186.
Description
A few sentences describing the changes proposed in this pull request.
Types of changes
./runtests.sh -f -u --net --coverage
../runtests.sh --quick --unittests --disttests
.make html
command in thedocs/
folder.