feat(api): add missing_ok: bool param to Table.drop()
#10816
Conversation
| This is useful if you need a column to not exist, | ||
| but you don't know if it does or not. | ||
|
|
||
| >>> t.drop("column_that_must_not_exist_afterwards") # doctest: +SKIP |
There was a problem hiding this comment.
I left out the specific error because I am worried that is going to change and become stale.
I would only want to show the actual error type if we remove the doctest: +SKIP marker, but I thought the extra boilerplate to actually be able to catch the error was not worth it. Open to adjusting this though.
ibis/expr/types/relations.py
Outdated
| if missing_ok: | ||
| return [] | ||
| raise | ||
| except AttributeError as e: |
There was a problem hiding this comment.
Needing to catch these two different kinds of exceptions here, and them being not very specific, I think really demonstrates the need for #10412
| @@ -2401,6 +2403,28 @@ def drop(self, *fields: str | Selector) -> Table: | |||
| │ Torgersen │ 19.3 │ 193 │ 3450 │ female │ … │ | |||
| └───────────┴───────────────┴───────────────────┴─────────────┴────────┴───┘ | |||
|
|
|||
| If a given column does not exist, then by default an error is raised. | |||
There was a problem hiding this comment.
| If a given column does not exist, then by default an error is raised. | |
| If a column does not exist, then by default an error is raised. |
| This is useful if you need a column to not exist, | ||
| but you don't know if it does or not. |
There was a problem hiding this comment.
| This is useful if you need a column to not exist, | |
| but you don't know if it does or not. |
I don't this this bit of explanation is necessary.
|
|
||
| >>> t.drop("column_that_must_not_exist_afterwards") # doctest: +SKIP | ||
| Traceback (most recent call last): | ||
| ... |
ibis/expr/types/relations.py
Outdated
| return [] | ||
| raise | ||
| except AttributeError as e: | ||
| if "has no attribute" in str(e): |
There was a problem hiding this comment.
Checking for substrings in error messages is extremely fragile. The implementation of this functionality should basically never depend on the specific content an informational error message.
There was a problem hiding this comment.
Additionally, binding for every column is very expensive for very wide tables.
Instead, what we should do is the following:
- Introduce
FieldNotFoundErroras you say - Allow bind (perhaps through an option to
_fast_bind?) to return an optimistic (possibly) subset: basically whatever columns are found fromfields.
resolves #10709