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

test: allow for tests to be run without Polars / PyArrow/ pandas installed #1726

Open
MarcoGorelli opened this issue Jan 5, 2025 · 9 comments
Labels
high priority Your PR will be reviewed very quickly if you address this internal tests

Comments

@MarcoGorelli
Copy link
Member

It would be nice to be able to do say pytest --constructors=pandas and not need Polars installed

To do this, we should replace import polars with polars = pytest.importorskip('polars') only in the places where it's truly necessary to import Polars

@mgorny
Copy link
Contributor

mgorny commented Jan 29, 2025

Yes, please. I wanted to package narwhals for Gentoo, but can't do that because the test suite needs polars, and polars simply cannot be packaged because they require a nightly Rust compiler.

Unfortunately, this isn't going to be trivial. From a quick look I've done in the morning, polars are used directly in a lot of fixtures — I guess using something akin to pytest-lazy-fixtures may be helpful.

@MarcoGorelli
Copy link
Member Author

Hi @mgorny !

How would you be looking to run the test suite in Gentoo? If you were able to run the full test suite for say pandas and duckdb, would that be enough? If so, I think it would be quite easy for us to allow

pytest --constructors=pandas,duckdb

without needing polars installed

@FBruzzesi
Copy link
Member

FBruzzesi commented Jan 29, 2025

Hey @mgorny, thanks for the feedback! These are very useful to understand what to prioritize!
I think #1843 was close to achieving it. I will spend more time into it or feel free to pick it up.

@MarcoGorelli I think the issue is that there are some top level imports of polars in the test suite, which would still require installation

@MarcoGorelli
Copy link
Member Author

MarcoGorelli commented Jan 29, 2025

I think just using polars = pytest.importorskip('polars') in the (relatively few) tests which specifically use Polars should already be pretty close, and would be a relatively small lift?

@mgorny
Copy link
Contributor

mgorny commented Jan 29, 2025

Ideally, we'd want to skip duckdb too, since we don't have it packaged either. pandas and pyarrow are fine for us. And yes, passing --constructors= would be good.

@MarcoGorelli
Copy link
Member Author

MarcoGorelli commented Jan 29, 2025

Cool, I made a start on this in the importorskip-polars branch: https://github.com/narwhals-dev/narwhals/tree/importorskip-polars

Commit diff: 32b3175

Seems to work fine:

$ python -c 'import polars'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ModuleNotFoundError: No module named 'polars'
$ pytest tests/group_by_test.py --constructors=pandas
================================================= test session starts =================================================
platform linux -- Python 3.12.6, pytest-8.3.4, pluggy-1.5.0
Using --randomly-seed=176162433
rootdir: /home/marcogorelli/polars-api-compat-dev
configfile: pyproject.toml
plugins: hypothesis-6.123.11, cov-6.0.0, env-1.1.5, randomly-3.16.0
collected 33 items                                                                                                    

tests/group_by_test.py ....................s............                                                        [100%]

============================================ 32 passed

There's almost 30 test files left where the polars import needs moving inside the test

If we could then do the same for DuckDB, and verify that pytest --constructors=pandas,pyarrow runs without needing Polars nor DuckDB installed, that should be all you need. It wouldn't hit 100% coverage of course, but I think that's OK, most tests would run, and I think enough to give confidence that packaging happened correctly

@MarcoGorelli MarcoGorelli added good first issue high priority Your PR will be reviewed very quickly if you address this labels Jan 29, 2025
@mgorny
Copy link
Contributor

mgorny commented Jan 30, 2025

I can spend some time on this today. Do you have some preference on how to handle parametrization?

@pytest.mark.parametrize("dtype", [pl.String, pl.String()])

I can't immediately think of a clean way of doing this. But I'll handle the more trivial cases at least.

@MarcoGorelli
Copy link
Member Author

I think it's fine to remove the parametrisation and loop over dtype in the test there, and skip it if polars isn't installed

@mgorny
Copy link
Contributor

mgorny commented Jan 30, 2025

I think it's fine to remove the parametrisation and loop over dtype in the test there, and skip it if polars isn't installed

Oh, that's one approach I didn't think of! I suppose I'm "parametrize" everything kind of person :-).

mgorny added a commit to mgorny/narwhals that referenced this issue Mar 18, 2025
Scope dataframe library imports in `conftest.py` to make it possible
to run the test suite with a subset of available dataframe libraries.
This makes it possible to run at least a subset of the test suite.

Part of bug narwhals-dev#1726
mgorny added a commit to mgorny/narwhals that referenced this issue Mar 18, 2025
This is a batch of changes aiming at making it possible to run the test
suite without `polars` installed.  Whenever easily feasible, the use
of `pyarrow` and `pandas` was made optional as well.

Part of bug narwhals-dev#1726
MarcoGorelli pushed a commit that referenced this issue Mar 18, 2025
…onftest.py` (#2241)

* test: Work towards making polars optional for tests

This is a batch of changes aiming at making it possible to run the test
suite without `polars` installed.  Whenever easily feasible, the use
of `pyarrow` and `pandas` was made optional as well.

Part of bug #1726

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
mgorny added a commit to mgorny/narwhals that referenced this issue Mar 19, 2025
Split and/or skip tests requiring `polars` when they are unavailable.
Whenever that was a low-hanging fruit, also imports of `pandas`
and `pyarrow` were made optional.

Part of bug narwhals-dev#1726
mgorny added a commit to mgorny/narwhals that referenced this issue Mar 19, 2025
Split and/or skip tests requiring `polars` when they are unavailable.
Whenever that was a low-hanging fruit, also imports of `pandas`
and `pyarrow` were made optional.

Part of bug narwhals-dev#1726
MarcoGorelli pushed a commit that referenced this issue Mar 19, 2025
)

* test: Fix more tests to work without polars (and pandas, pyarrow)

Split and/or skip tests requiring `polars` when they are unavailable.
Whenever that was a low-hanging fruit, also imports of `pandas`
and `pyarrow` were made optional.

Part of bug #1726

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Fix typing

* Refactor all optional test imports

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Convert duckdb and ibis instances

* Also pa_csv

* Ignore type hints on more tested-invalid calls

* Fix ruff

* Workaround ruff in `ibis_test.py`

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Dan Redding <[email protected]>
mgorny added a commit to mgorny/narwhals that referenced this issue Mar 19, 2025
Split and/or skip tests requiring `polars` when they are unavailable.
Whenever that was a low-hanging fruit, also imports of `pandas`
and `pyarrow` were made optional.

Part of bug narwhals-dev#1726
MarcoGorelli pushed a commit that referenced this issue Mar 19, 2025
)

* test: Fix yet more tests to work without polars (pandas, pyarrow)

Split and/or skip tests requiring `polars` when they are unavailable.
Whenever that was a low-hanging fruit, also imports of `pandas`
and `pyarrow` were made optional.

Part of bug #1726

* Cover `tests/expr_and_series/arithmetic_test.py`

* Port remaining `tests/expr_and_series` tests

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Try adding empty `else` branch to `from_dict_test`

* Cover `collect_test`

* Try extend() approach

* Update tests/from_dict_test.py

Co-authored-by: Dan Redding <[email protected]>

* Fix coverage in `tests/translate/from_native_test.py`

* be quiet coverage

https://github.com/narwhals-dev/narwhals/actions/runs/13951644267/job/39052438974?pr=2250

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Dan Redding <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high priority Your PR will be reviewed very quickly if you address this internal tests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants