Added `isnone()` function #801

ilongin · 2025-01-07T14:31:15Z

isnone(...) function returns True if column or literal value is None / NULL, otherwise False

Note that this doesn't currently work for Clickhouse where we don't have nullable columns. We should decide if we want to introduce nullable columns and if yes, this will work there as well.

UPDATE:
In addition, this PR extends case() function and related codebase to accept nested functions in:

Case condition - value (both places)
Case else_ value

This will allow usage of functions inside other functions that are based on case like ifelse or isnone as well. We also will have ability to do nested cases etc.

codecov · 2025-01-07T14:43:03Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.46%. Comparing base (2364ac2) to head (5644531).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #801      +/-   ##
==========================================
+ Coverage   87.44%   87.46%   +0.01%     
==========================================
  Files         128      128              
  Lines       11369    11383      +14     
  Branches     1535     1538       +3     
==========================================
+ Hits         9942     9956      +14     
  Misses       1048     1048              
  Partials      379      379

Flag	Coverage Δ
datachain	`87.40% <100.00%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

shcheklein · 2025-01-07T16:21:12Z

src/datachain/func/conditional.py

+
+    Example:
+        ```py
+        dc.mutate(test=isnone("value"))


can we make a more reasonable example - filter, or some if condition ...

I realized that I need to support nested case which is used by ifelse and isnone for this. I'm adding needed changes into this PR.

Added better example

shcheklein · 2025-01-07T16:22:53Z

src/datachain/func/conditional.py

+    Args:
+        col (str | Column | literal): Column or literal to check if None.
+            If a string is provided, it is assumed to be the name of the column.
+            If a literal is provided, it is assumed to be a string literal.


do we need to specify literals separately? why?

Removed it. Technically someone can provide just string literal but that doesn't make much sense, except for unit tests. Columns are the one to be used here

shcheklein · 2025-01-07T16:23:23Z

src/datachain/func/conditional.py

+
+def isnone(col: Union[str, Column]) -> Func:
+    """
+    Returns True if column value or literal is None, otherwise False


or literal - what does it mean? could you give an example please?

can we pass an expression btw ? e.f. isnone(ifelse(...))? if not, why not?

We should be able to pass expression. As I wrote in above response, I need to do some improvements to general case function to be able to have nested ones for mentioned examples to work

Added ability to use nested functions inside case or ifelse (which is based on case)

cloudflare-workers-and-pages · 2025-01-09T14:53:09Z

Deploying datachain-documentation with Cloudflare Pages

Latest commit:	`c25e359`
Status:	✅ Deploy successful!
Preview URL:	https://14e74cac.datachain-documentation.pages.dev
Branch Preview URL:	https://ilongin-800-isnone-function.datachain-documentation.pages.dev

View logs

src/datachain/func/conditional.py

shcheklein · 2025-01-10T17:19:40Z

src/datachain/func/conditional.py

-def case(*args: tuple[BinaryExpression, CaseT], else_=None) -> Func:
+def case(
+    *args: tuple[Union[ColumnElement, Func], CaseT], else_: Optional[CaseT] = None
+) -> Func:
    """
    Returns the case function that produces case expression which has a list of
    conditions and corresponding results. Results can only be python primitives


is it still true? can result now be a Func?

Yes, all methods here return Func which is essentially a wrapper around sqlalchemy functions like case which produces case construct

I mean this part Results can only be python primitives ..., sorry

src/datachain/func/conditional.py

shcheklein · 2025-01-10T17:28:00Z

src/datachain/func/conditional.py

+        if_val: (str | int | float | complex | bool, Func): value for true
+            condition outcome
+        else_val: (str | int | float | complex | bool, Func): value for false condition
+            outcome

    Returns:
        Func: A Func object that represents the ifelse function.

    Example:
        ```py
        dc.mutate(


let's add another example with a column expression?

also an examples with a results as an expression?

Do we need that many examples? All other functions have exactly one example even though there can be more as well like the version with column expression as you stated.

to my mind yes, examples is the most valuable part usually since you can get an idea of what is actually possible. We need more simple examples everywhere.

shcheklein · 2025-01-10T19:47:09Z

src/datachain/func/conditional.py

+    """
+    Returns True if column value is None, otherwise False
+    Args:
+        col (str | Column): Column to check if it's None or not.


consistency

I'm trying to figure out but not sure what is wrong in this part of docs?

e.e. in some cases I saw it starts with lower case, sometimes I saw - before the descriptions, sometimes there was no period at the end, sometimes it's a new line, etc

and all of this within this single PR

and it also applies to the description, e.g. we don't have period here - is it always the case?

shcheklein · 2025-01-10T19:49:09Z

src/datachain/func/conditional.py

+        # if string, it is assumed to be the name of the column
+        col = C(col)
+
+    return case((col == None, True), else_=False)  # type: ignore [arg-type]  # noqa: E711


why do we need # type: ignore [arg-type] # noqa: E711?

will users who will use case also have this issue with types?

arg-type is not needed indeed... probably was needed in some previous iterations and stopped being needed after some refactoring so it's a leftover.
Regarding E711 ... ruff is complaining about comparing with None with == and says it should be is None, but since this is SQL construct it should actually be like this (I've tried with is None but doesn't work)

is there an analog of our isnone for this case? I wonder how should people use it raises an obvious warning ...

shcheklein · 2025-01-10T22:28:38Z

tests/unit/test_func.py

+
+
+@pytest.mark.parametrize("col", ["val", C("val")])
+@skip_if_not_sqlite


does it mean that we can't even run the operation itself on CH - it won't work at all? or will it run, just returning always False?

I will double check but from what I remember it always return False so wrong result, which is even worse than failing IMO. Dmitry created task in Studio to allow nullable columns in CH so this should not be a problem in the future.
I'm not 100% sure if I should merge this PR now though, as I don't like to leave it in review for so long due conflicts etc, but on the other hand it doesn't really work for CH...so I guess it will idle here until we allow nullable in CH after all.

I think it's fine to merge ... it always returns False for now - and I would test for it (so that function can run). We just don't support nullables there at all ...

tests/unit/test_func.py

shcheklein · 2025-01-10T22:31:34Z

src/datachain/func/func.py

@@ -423,7 +430,7 @@ def get_db_col_type(signals_schema: "SignalSchema", col: ColT) -> "DataType":
        return sql_to_python(col)

    return signals_schema.get_column_type(
-        col.name if isinstance(col, ColumnElement) else col
+        col.name if isinstance(col, ColumnElement) else col  # type: ignore[arg-type]


why do we have this ignore here?

shcheklein · 2025-01-11T02:15:38Z

src/datachain/func/conditional.py

-        else_ (str | int | float | complex | bool): else value in case expression
+        args (tuple((ColumnElement, Func), (str | int | float | complex | bool, Func))):
+            Tuple of condition and values pair.
+        else_ (str | int | float | complex | bool, Func): else value in case


is it optional? can we say optional else value. What happens by default?

(no in case expression since it's clear from the context)

shcheklein · 2025-01-11T02:15:56Z

src/datachain/func/conditional.py

-    and values for true and false outcome. Results can only be python primitives
-    like string, numbes or booleans. Result type is inferred from the values.
+    and values for true and false outcome. Results can be one of python primitives
+    like string, numbes or booleans, but can also be nested functions.


numbes -> numbers

shcheklein · 2025-01-11T02:19:33Z

src/datachain/func/conditional.py

 from datachain.sql.functions import conditional

 from .func import ColT, Func

-CaseT = Union[int, float, complex, bool, str]
+CaseT = Union[int, float, complex, bool, str, Func]


are there tests for Func values btw? can we add them?

ilongin added 2 commits January 7, 2025 14:42

added isnone function and tests

8d35c7e

merging with main and exporting function

7a19674

ilongin temporarily deployed to internal January 7, 2025 14:31 — with GitHub Actions Inactive

ilongin linked an issue Jan 7, 2025 that may be closed by this pull request

Add isnone() function #800

Open

ilongin marked this pull request as draft January 7, 2025 14:31

removing prints

fddab8a

ilongin temporarily deployed to internal January 7, 2025 14:33 — with GitHub Actions Inactive

shcheklein reviewed Jan 7, 2025

View reviewed changes

ilongin added 2 commits January 8, 2025 11:27

Merge branch 'main' into ilongin/800-isnone-function

be065d4

added nested case ability

ba5d315

ilongin temporarily deployed to internal January 9, 2025 14:53 — with GitHub Actions Inactive

fixing typing of conditional functions

40ea99f

ilongin temporarily deployed to internal January 10, 2025 09:13 — with GitHub Actions Inactive

added fix for columns - literals inside case

a696435

ilongin temporarily deployed to internal January 10, 2025 10:07 — with GitHub Actions Inactive

ilongin added 2 commits January 10, 2025 12:14

changing docs

9038cc0

Merge branch 'main' into ilongin/800-isnone-function

5644531

ilongin marked this pull request as ready for review January 10, 2025 11:15

ilongin temporarily deployed to internal January 10, 2025 11:15 — with GitHub Actions Inactive

ilongin requested review from shcheklein, dreadatour, amritghimire, mattseddon and skshetry January 10, 2025 11:15

shcheklein reviewed Jan 10, 2025

View reviewed changes

src/datachain/func/conditional.py Outdated Show resolved Hide resolved

shcheklein reviewed Jan 10, 2025

View reviewed changes

src/datachain/func/conditional.py Outdated Show resolved Hide resolved

shcheklein reviewed Jan 10, 2025

View reviewed changes

src/datachain/func/conditional.py Outdated Show resolved Hide resolved

shcheklein reviewed Jan 10, 2025

View reviewed changes

tests/unit/test_func.py Show resolved Hide resolved

shcheklein reviewed Jan 10, 2025

View reviewed changes

ivan added 2 commits January 11, 2025 00:54

fixing docs

1f9d382

fixing docs

c25e359

ilongin deployed to internal January 11, 2025 00:18 — with GitHub Actions Active

shcheklein reviewed Jan 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added `isnone()` function #801

Added `isnone()` function #801

ilongin commented Jan 7, 2025 •

edited

Loading

codecov bot commented Jan 7, 2025 •

edited

Loading

shcheklein Jan 7, 2025

ilongin Jan 8, 2025

ilongin Jan 10, 2025

shcheklein Jan 7, 2025

ilongin Jan 10, 2025

shcheklein Jan 7, 2025

shcheklein Jan 7, 2025

ilongin Jan 8, 2025

ilongin Jan 10, 2025

cloudflare-workers-and-pages bot commented Jan 9, 2025 •

edited

Loading

shcheklein Jan 10, 2025

ilongin Jan 10, 2025

shcheklein Jan 11, 2025

shcheklein Jan 10, 2025

ilongin Jan 10, 2025

shcheklein Jan 11, 2025

shcheklein Jan 10, 2025

ilongin Jan 10, 2025

shcheklein Jan 11, 2025

shcheklein Jan 10, 2025

ilongin Jan 10, 2025

shcheklein Jan 11, 2025

shcheklein Jan 10, 2025

ilongin Jan 11, 2025

shcheklein Jan 11, 2025

shcheklein Jan 10, 2025

shcheklein Jan 11, 2025

shcheklein Jan 11, 2025

shcheklein Jan 11, 2025



		@pytest.mark.parametrize("col", ["val", C("val")])
		@skip_if_not_sqlite

Added isnone() function #801

Are you sure you want to change the base?

Added isnone() function #801

Conversation

ilongin commented Jan 7, 2025 • edited Loading

codecov bot commented Jan 7, 2025 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cloudflare-workers-and-pages bot commented Jan 9, 2025 • edited Loading

Deploying datachain-documentation with Cloudflare Pages

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Added `isnone()` function #801

Added `isnone()` function #801

ilongin commented Jan 7, 2025 •

edited

Loading

codecov bot commented Jan 7, 2025 •

edited

Loading

cloudflare-workers-and-pages bot commented Jan 9, 2025 •

edited

Loading