search

narwhals-dev / narwhals

Lightweight and extensible compatibility layer between Polars, pandas, cuDF, Modin, and more!
https://narwhals-dev.github.io/narwhals/
MIT License
218 stars 31 forks source link

ci: Added Version #316

Closed DeaMariaLeon closed 1 week ago

DeaMariaLeon commented 1 week ago

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

Related issues

Checklist

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

Adapted from sklearn

DeaMariaLeon commented 1 week ago

@MarcoGorelli Is it OK to have the dependencies hard-coded on narwhals/functions.py lines 69-79?

deps = [
        "covdefaults",
        "pandas",
        "polars",
        "pre-commit",
        "pyarrow",
        "pytest",
        "pytest-cov",
        "hypothesis",
        "scikit-learn",
    ]

The coverage test is failing. Apparently the missing lines (that should be tested) are the ones that are executed when there is a PackageNotFoundError. I wonder if I have to add a smart way to check the dependencies in _get_deps_info (lines above). If that's not the case, I haven't found yet how to test for a package not found. 

for modname in deps:
        try:
            deps_info[modname] = version(modname)
        except PackageNotFoundError:  # noqa: PERF203
            deps_info[modname] = ""
    return deps_info
FBruzzesi commented 1 week ago

Hey @DeaMariaLeon, I took a quick look.

For the failing test, I think adding cudf to the list of libraries would fix it as ci/cd does not installs cudf, hence it should enter such except block :) On the other hand it is a reasonable choice as a user could use narwhals with cudf in the first place

MarcoGorelli commented 1 week ago

thanks for doing this!

I'd suggest just reporting __version__ for each package. I think the only ones we need are:

These should all have __version__, if we come across a package later which doesn't, we deal with that then

DeaMariaLeon commented 1 week ago

@MarcoGorelli do you mean using __version__ instead of from importlib.metadata import version? To use __version__ I then should use your functions get_polars(), get_pandas(), right? If not, then the packages need to be imported, don't they? - I have not updated with your comment since I'm not sure. Just added the right packages.

Thanks @FBruzzesi, adding cudf seem to do the trick.