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

ci: Add darts to downstream tests #2118

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

FBruzzesi
Copy link
Member

What type of PR is this? (check all applicable)

  • πŸ’Ύ Refactor
  • ✨ Feature
  • πŸ› Bug Fix
  • πŸ”§ Optimization
  • πŸ“ Documentation
  • βœ… Test
  • 🐳 Other

Related issues

darts#2661 just got merged

Checklist

  • Code follows style guide (ruff)
  • Tests added
  • Documented the changes

If you have comments or can explain your changes, please do so below

Darts entire test suite takes a bit too much for our use case - Narwhals is used in TimeSeries class, that's what I am adding as test here

@FBruzzesi FBruzzesi added the ci label Feb 28, 2025
@FBruzzesi
Copy link
Member Author

Hey @dennisbader and @authierj, we try to run test suites for downstream libraries that use narwhals in our CI, to make sure that we don't break anything by accident.

As mentioned in the description, darts test suite takes ~30mins, but narwhals is used just in TimeSeries test. Do you think that's enough for now to test?

@dennisbader
Copy link

Hi @FBruzzesi, that's very nice πŸš€

I'd also add the following tests (they're very lightweight):

  • test_timeseries_multivariate.py, contains narwhals related checks for multivariate DataFrames.
  • test_static_covariates.py, contains narwhals related checks for DataFrames and static covariates.
  • we'll be adding checks using polars as backend soon. Polars will only be an optional dependency and not be part of our core.txt requirements file (nor dev.txt). But it looks like you already install it for the tests, so all good πŸ‘

@FBruzzesi
Copy link
Member Author

I'd also add the following tests (they're very lightweight):

  • test_timeseries_multivariate.py, contains narwhals related checks for multivariate DataFrames.
  • test_static_covariates.py, contains narwhals related checks for DataFrames and static covariates.

Thanks @dennisbader , I just added these two tests as well

  • we'll be adding checks using polars as backend soon. Polars will only be an optional dependency and not be part of our core.txt requirements file (nor dev.txt). But it looks like you already install it for the tests, so all good πŸ‘

Actually we don't install polars if not needed - I added it now, with a TODO note to use darts specification when you are going to introduce it in CI/some requirements

@FBruzzesi FBruzzesi changed the title ci: add darts to downstream tests ci: Add darts to downstream tests Mar 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants