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

refactor: remove ibis.NA #9311

Closed
ncclementi opened this issue Jun 4, 2024 · 4 comments · Fixed by #9344
Closed

refactor: remove ibis.NA #9311

ncclementi opened this issue Jun 4, 2024 · 4 comments · Fixed by #9344
Assignees
Labels
deprecation Issues or PRs related to deprecating APIs ux User experience related issues

Comments

@ncclementi
Copy link
Contributor

Opening an issue to discuss a separate the refactor of ibis.NA because doing it as part of #9300 will get messy.

To discuss:
Does removing ibis.NAimply:

  1. Replace ibis.NA for ibis.null()
  2. Get rid of untyped nulls all together, as in not use ibis.null() but just None
@jcrist
Copy link
Member

jcrist commented Jun 4, 2024

Another option is to rename it to ibis.NULL, but I'd (slightly) vote for option 1.

I'm not really sure what option 2 means - in many cases APIs that accept None should implicitly coerce it to a literal NULL, but not in all cases. There will always cases where calling ibis.null() (or ibis.literal(None), which is the same thing) will be necessary or more ergonomic.

@cpcloud
Copy link
Member

cpcloud commented Jun 4, 2024

I don't think can we remove ibis.null(), it accepts a dtype and can create a typed null which is necessary and/or useful in some cases.

I would vote for the following:

  1. Remove NA
  2. Don't add NULL
  3. Use None wherever untyped NULLs previously worked.

ibis.null(...) remains unchanged.

@cpcloud
Copy link
Member

cpcloud commented Jun 4, 2024

As far implementation goes, we can basically search and replace ibis.NA with ibis.null(). If folks want an untyped NULL, that's None (after these changes).

@jcrist jcrist added ux User experience related issues deprecation Issues or PRs related to deprecating APIs labels Jun 5, 2024
@ncclementi
Copy link
Contributor Author

For implementation purposes, do we want to deprecate ibis.NAor remove it as a breaking change. If it's a deprecation, how do we deprecate ibis.NA = null() the @deprecated seems to only work for methods/functions.

This was referenced Jun 6, 2024
ncclementi added a commit that referenced this issue Jun 11, 2024
- Closes #9311 

This PR removes ibis.NA, replacing it appearances for ibis.null()

---------

Co-authored-by: Jim Crist-Harif <jcristharif@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation Issues or PRs related to deprecating APIs ux User experience related issues
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants