Skip to content
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

feat(api): make ibis.literal() idempotent #9469

Closed
1 task done
NickCrews opened this issue Jun 28, 2024 · 4 comments
Closed
1 task done

feat(api): make ibis.literal() idempotent #9469

NickCrews opened this issue Jun 28, 2024 · 4 comments
Labels
feature Features or general enhancements

Comments

@NickCrews
Copy link
Contributor

Is your feature request related to a problem?

Inspired by #9458 (comment)

I occasionally "just want an ibis expression, I'm not sure what I was handed". For example, in the above issue, when trying to construct an ops.Array value, the members handed to me might be python literals, or ibis expressions. I want to cast them to the datatype I want, so they need to be ibis values, I can't do that casting on python vals.

This is similar in spirit to my current PRs which make ibis.map(), ibis.array(), and ibis.struct() idempotent.

What is the motivation behind your request?

No response

Describe the solution you'd like

def literal(value, type):
    if isinstance(value, (ibis.Expr, ibis.Deferred)):
        return value
    # the rest of the existing logic below

Currently, we explicitly disallow ibis.Exprs as inputs:

if isinstance(value, Expr):
node = value.op()
if not isinstance(node, ops.Literal):
raise TypeError(f"Ibis expression {value!r} is not a Literal")
if type is None or node.dtype.castable(dt.dtype(type)):
return value
else:
raise TypeError(
f"Ibis literal {value!r} cannot be safely coerced to datatype {type}"
)

so presumably there is a reason this hasn't been done yet? Any recollection what the reason was?

What version of ibis are you running?

main

What backend(s) are you using, if any?

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@NickCrews NickCrews added the feature Features or general enhancements label Jun 28, 2024
@cpcloud
Copy link
Member

cpcloud commented Jun 28, 2024

I occasionally "just want an ibis expression, I'm not sure what I was handed"

ibis.literal is not the API for that. Does it really make sense to pass arbitrary expressions to ibis.literal?

@NickCrews
Copy link
Contributor Author

I agree the name "literal" isn't great, I'm not wedded to stuffing that functionality in there, but is there some other good way of solving my problem? I just updated my OP with a link to a real implementation that I think could benefit from this functionality. IDK, maybe there is a way to avoid needing this functionality all together.

I think I understand your concern with stuffing too much functionality into one place, but I feel like when it is just adding idempotency, that is a lot less to worry about as opposed to eg accepting xarray values or dask values?

@cpcloud
Copy link
Member

cpcloud commented Jul 3, 2024

Is it a big lift to just keep a function around that does this in whatever code you're writing that needs this behavior?

@gforsyth
Copy link
Member

Going to close this as unplanned for now, @NickCrews -- I think we can keep the concerns in the linked PR scoped to arrays, maps, and structs.

@gforsyth gforsyth closed this as not planned Won't fix, can't repro, duplicate, stale Jul 17, 2024
@github-project-automation github-project-automation bot moved this from backlog to done in Ibis planning and roadmap Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Features or general enhancements
Projects
Archived in project
Development

No branches or pull requests

3 participants