Skip to content

Implement missing Expr methods聽#35

Open

Description

@MarcoGorelli

In https://github.com/narwhals-dev/narwhals-daft/blob/main/narwhals_daft/expr.py there's a list of what's not_implemented for Daft

Any help moving forwards with these would be greatly appreciated 馃檹

Task is:

  1. Follow the setup instructions on the README
  2. Implement some method
  3. You can run the tests for that method with run_test.py. E.g.: python run_tests.py -k is_unique to run the is_unique tests
  4. Run python update_run_tests.py when you're done, and hopefully the list in run_tests.py will shorten itself!

Feel free to ask for help if anything trips you up

Activity

  1. raisadz commented on Nov 13, 2025

    @raisadz
    Contributor

    I will be looking at implementing functions in the string namespace in the next few days

  2. MarcoGorelli commented on Nov 13, 2025

    @MarcoGorelli
    MemberAuthor

    thanks! i'd suggesting with just adding a single one (say, len_chars?) and marking all the others as not_implemented for now

  3. ym-pett commented on Nov 13, 2025

    @ym-pett
    Collaborator

    had a look at remaining functions & I'm going to attempt to implement mode tomorrow. 馃

    update: closed this PR for the time being as I couldn't quite figure out how not to use something which triggers a computation; come to think of it, this would be a good topic for the study group :)

    In the meantime, I've added a request for additional descriptive statistics with daft in their existing discussion on having more aggregations: Eventual-Inc/Daft#485

  4. raisadz commented on Nov 18, 2025

    @raisadz
    Contributor

    @ym-pett I will be happy if you want to take some of the string methods, just check this PR first #64. Also, I will try to work on slice tomorrow

  5. ym-pett commented on Nov 19, 2025

    @ym-pett
    Collaborator

    thanks @raisadz, I'd like to try .to_titlecase().

    Oh no it's one of those that don't exist in daft, I will try, can't guarantee 馃槄

  6. MarcoGorelli commented on Nov 19, 2025

    @MarcoGorelli
    MemberAuthor

    that one might be doable by following the same workaround we do in narwhals for sql / pyspark / duckdb

  7. ym-pett commented on Nov 19, 2025

    @ym-pett
    Collaborator

    yes, using analogy I've got something that works outside of Narwhals on a toy df but I'm struggling to incorporate it into "Narwhals language". I will try some more and may well ask questions in a draft PR. Thanks Marco!

  8. raisadz commented on Nov 19, 2025

    @raisadz
    Contributor

    I will take a look at str.replace_all next

  9. ym-pett commented on Nov 20, 2025

    @ym-pett
    Collaborator

    I was thinking of looking at the contains method in expr_str next.

  10. ym-pett commented on Nov 25, 2025

    @ym-pett
    Collaborator

    update on contains method:

    I've been investigating it for a couple of days now and get why the tests are failing with the native daft which doesn't have a way to pass a regex. On top of that, I can see in the tests that some of the backends are specified; at least in tests/expr_and_series/list/contains_test I see "dask", "modin", "cudf", "pyarrow", "pandas" and in the tests/expr_and_series/str/contains_test I see "cudf" specified but that might be to not run with/fail with that backend.

    1. Am I right in thinking even if I find a way of detecting that the user wants to use a regex, the tests themselves might need to be adapted to daft, which we don't want to do?

    2. I feel like sql / pyspark / duckdb modules are quite similar to daft, how far does it make sense to take what's implemented there as guidance? If something isn't implemented, is it fair to assume that there are some major technical hurdles, or is is it more likely to be coincidence and just hasn't been done yet?

    3. I'm thinking of looking for something else to implement, unless if you think it's worth pushing on with the contains method?

    Thanks for any advice you might have :)

  11. MarcoGorelli commented on Nov 25, 2025

    @MarcoGorelli
    MemberAuthor

    i haven't taken a close look at contains yet, but there's probably quite a few dt methods that should be feasible

    If something isn't implemented, is it fair to assume that there are some major technical hurdles, or is is it more likely to be coincidence and just hasn't been done yet?

    yeah probably a good assumption

  12. ym-pett commented on Nov 25, 2025

    @ym-pett
    Collaborator

    thanks Marco! I'll have a look & drop which method I'll take here.

  13. raisadz commented on Nov 25, 2025

    @raisadz
    Contributor

    update on contains method:

    I've been investigating it for a couple of days now and get why the tests are failing with the native daft which doesn't have a way to pass a regex. On top of that, I can see in the tests that some of the backends are specified; at least in tests/expr_and_series/list/contains_test I see "dask", "modin", "cudf", "pyarrow", "pandas" and in the tests/expr_and_series/str/contains_test I see "cudf" specified but that might be to not run with/fail with that backend.

    1. Am I right in thinking even if I find a way of detecting that the user wants to use a regex, the tests themselves might need to be adapted to daft, which we don't want to do?
    2. I feel like sql / pyspark / duckdb modules are quite similar to daft, how far does it make sense to take what's implemented there as guidance? If something isn't implemented, is it fair to assume that there are some major technical hurdles, or is is it more likely to be coincidence and just hasn't been done yet?
    3. I'm thinking of looking for something else to implement, unless if you think it's worth pushing on with the contains method?

    Thanks for any advice you might have :)

    When I had a quick look at contains before I also noticed that they only accept strings rather than regex. Other string methods that are left might need several workarounds. @ym-pett I agree with Marco that you can take a look at some other non-implemented namespaces as there might be a few Daft functions that can work out-of-the-box. I haven't looked at them yet

  14. ym-pett commented on Nov 25, 2025

    @ym-pett
    Collaborator

    ok cool, thanks @raisadz ! I've started looking at to_datetime but I've not investigated how it works in daft compared to nw, will let you know how I get on.

  15. 9 remaining items

  16. ym-pett commented on Jan 16, 2026

    @ym-pett
    Collaborator

    thanks @raisadz! Ignore my earlier (deleted) comment - someone has already done it :)

  17. MarcoGorelli commented on Jan 16, 2026

    @MarcoGorelli
    MemberAuthor

    Thanks!

    Btw we agreed a scheduled run to auto update the submodule but accidentally broke the typing check in doing so, for now id suggest ignoring it (or adding a couple of not_implemented to fix it?)

  18. ym-pett commented on Jan 16, 2026

    @ym-pett
    Collaborator

    Hi @MarcoGorelli , didn't see your message in time, but did the not_implemented thing :)

    Can I check I understand: the scheduled auto-update means it'll be done automatically at a fixed time in the week & we can ignore doing the updates from now on?

  19. MarcoGorelli commented on Jan 16, 2026

    @MarcoGorelli
    MemberAuthor

    Thanks!

    Yeah it should happen automatically each Thursday, a PR should get auto-created and we just need to merge. Hopefully, we'll see next week 馃槅

  20. ym-pett commented on Jan 16, 2026

    @ym-pett
    Collaborator

    very cool!

  21. ym-pett commented on Feb 2, 2026

    @ym-pett
    Collaborator

    I'm looking around for things I can do easily, in the meantime thought I should also ask if there is there anything you'd like done but haven't got time for @raisadz or @MarcoGorelli?

    (I don't quite see what work would be involved in the other issues apart from #79, which I think is done for the moment :))

  22. MarcoGorelli commented on Feb 2, 2026

    @MarcoGorelli
    MemberAuthor

    thanks! i think

        "test_filter_windows",
        "test_filter_windows_over",
    

    might be doable if you fancy it? feel free to ping if it's not clear how

  23. ym-pett commented on Feb 2, 2026

    @ym-pett
    Collaborator

    great thanks!! In the meantime I've started on trying contains for the list_expression, but if I hit a wall I'll probably quickly switch over to those 馃槄

  24. ym-pett commented on Feb 2, 2026

    @ym-pett
    Collaborator

    yep, hit that wall, going to switch over to "test_filter_windows" :)

    @MarcoGorelli, if you have time, could you tell me what's needed here: I can see that for narwhals_daft dataframe.filter is already implemented and the test already exists in the narwhals submodule, is it a refactor you had in mind?

    update: fresh eyes, I think I now get it, filter in the expr modules is meant, will try that now. Sorry just couldn't see it last night.

  25. ym-pett commented on Feb 9, 2026

    @ym-pett
    Collaborator

    It may be embarrassing to say that I was barking up completely the wrong tree with the draft PR I tried, but I found it super useful to learn more about why expr.filter can't work with lazy-only backends and I now understand more about how nw does it for the other backends.

    I should be able to figure out what "test_filter_windows" needs now.

  26. MarcoGorelli commented on Feb 9, 2026

    @MarcoGorelli
    MemberAuthor

    馃槃 sorry about that - yeah this one's about allowing for things like df.filter(nw.col('price') = nw.col('price').min()), so you'll need to define DataFrame._filter, i think if you take a look at the sql or duckdb code in narwhals it should provide a good starting point

  27. ym-pett commented on Feb 9, 2026

    @ym-pett
    Collaborator

    haha no worries at all it was so good for me to get lost, feel like I learned a lot!!

  28. ym-pett commented on Feb 9, 2026

    @ym-pett
    Collaborator

    @MarcoGorelli on draft PR #118, is the CI just generally failing or is there something I can do on my end?

  29. MarcoGorelli commented on Feb 19, 2026

    @MarcoGorelli
    MemberAuthor

    hey - i think if you fetch and merge upstream/main it might resolve itself? it looks like a network issue

  30. jonasdedden commented on Sep 4, 2026

    @jonasdedden
    Contributor

    PRs #137, #139, #140, #141, #142, #143, #144, #145 implement 15 new expressions, with roughly ~44 previously failing tests now green (hopefully 馃, at least locally they are 馃槄 ).

    Hinges on merging #138 first though. Left all new PRs on draft mode because of this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions