ibis icon indicating copy to clipboard operation
ibis copied to clipboard

feat: Support `type` arg to `ibis.array` and `ibis.map`

Open NickCrews opened this issue 1 year ago • 6 comments

Is your feature request related to a problem?

This is already supported for ibis.struct.

I want to create an array of type array<uint8>. ibis.array() doesn't support a type param, so if the type can't be inferred from the values, I can't use it:

  • If I want a NULL value, I have to use ibis.literal(None, type="array<uint8>").
  • If I want length-0 value, I have to use ibis.literal([], type="array<uint8>")
  • For longer-length arrays, I am able use ibis.array for the initial construction, but then I have to do a .cast() afterwards to ensure the type is correct: ibis.array([1,2]).type() shows array<int8>, so I have to do ibis.array([1,2]).cast("array<uint8>")

I haven't ever used map types, but I imagine it is a similar story.

It is annoying to use two different constructors depending on the length. There should be one way.

I could just use ibis.literal, but it's docstring says that constructing these complex types using it is going to be deprecated in a future release. If that is the case, then my workarounds have a shelf life. U

Describe the solution you'd like

add a type param to these three functions. It should look just like it does for struct, I like that API.

Then, either:

  • actually follow through with what the docstring says for literal and start the deprecation and removal timeline
  • remove that note on future deprecation. This was added in this commit, but I can't find a reasoning behind it.

What version of ibis are you running?

main

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

No response

Code of Conduct

  • [X] I agree to follow this project's Code of Conduct

NickCrews avatar Feb 08 '24 22:02 NickCrews

/take

chloeh13q avatar Mar 07 '24 16:03 chloeh13q

@NickCrews IIUC, your PR #8666 should also resolve this. If so, I will re-assign this issue to you.

chloeh13q avatar Mar 20 '24 21:03 chloeh13q

That would work. Also, that PR is turning into a bit of a doozy, if you wanted to help out there it would be appreciated!

NickCrews avatar Mar 20 '24 22:03 NickCrews

@NickCrews Sure, I can take a look!

chloeh13q avatar Mar 20 '24 22:03 chloeh13q

@NickCrews I noticed there are multiple PRs open, draft or closed that were aiming to tackle this issue, but it's unclear what's the status. Would it be possible to leave a comment here on what are the steps needed to get this one across the line?

ncclementi avatar Jul 19 '24 16:07 ncclementi

#9473 needs to be resolved for the array changes (which really is getting stalled by larger philosophical decisions that this PR is exposing), and then the linked map PR can be resumed.

NickCrews avatar Jul 19 '24 16:07 NickCrews