-
Notifications
You must be signed in to change notification settings - Fork 598
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
fix(mutate/select): ensure that unsplatted dictionaries work in mutate
andselect
APIs
#8014
Conversation
ACTION NEEDED Ibis follows the Conventional Commits specification for release automation. The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification. |
muta… …te
andselect
APIs
muta… …te
andselect
APIsmutate
andselect
APIs
6cb933f
to
faf7cb2
Compare
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.
Overall concept looks good to me, just a couple code nits/questions.
|
||
for expr in exprs: | ||
if isinstance(expr, Selector): | ||
new_exprs.extend(expr.expand(self)) |
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.
_ensure_expr
handles selector expansion, I think this case could be dropped and handled by the not-mapping branch below?
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.
Unfortunately it won't work because expr.expand(self)
returns a list
and so map(self._ensure_expr, ...)
returns an iterable of list
s and that isn't supported by Projector
.
We were not handling unsplatted dicts in mutate/select, this PR fixes that. Fixes #8013.