[tests][python-package] refactor list_to_1d_numpy test to run without pandas installed

[jmoralez]

Currently tests/python-package-test/test_basic.py::test_list_to_1d_numpy uses pd_Series within the parametrize calls, which fails when pandas isn't installed. This moves the series creation after checking that pandas is indeed installed and we can be sure that pd_Series(collection) will succeed.

Fixes #4638.

[strobelTha]

I fear this construct might be hard to understand if the reason behind it is unknown.
Would it be an option to not change the test at all, but adapt the dummy implementation for pandas in lightgbm/compat.py

Current:

try:
    from pandas import DataFrame as pd_DataFrame
    from pandas import Series as pd_Series
    from pandas import concat
    from pandas.api.types import is_sparse as is_dtype_sparse
    PANDAS_INSTALLED = True
except ImportError:
    PANDAS_INSTALLED = False

    class pd_Series:  # type: ignore
        """Dummy class for pandas.Series."""

        pass

    class pd_DataFrame:  # type: ignore
        """Dummy class for pandas.DataFrame."""

        pass

Proposal:

try:
    from pandas import DataFrame as pd_DataFrame
    from pandas import Series as pd_Series
    from pandas import concat
    from pandas.api.types import is_sparse as is_dtype_sparse
    PANDAS_INSTALLED = True
except ImportError:
    PANDAS_INSTALLED = False

    class pd_Series:  # type: ignore
        """Dummy class for pandas.Series."""
        def __init__(self, *args):
            pass

    class pd_DataFrame:  # type: ignore
        """Dummy class for pandas.DataFrame."""
        def __init__(self, *args):
            pass

[StrikerRUS]

I agree with @strobelTha . I believe that making all our dummy classes from compat.py creatable with any combination of arguments by adding catch-all constructor will be useful

class pd_Series:  # type: ignore
    """Dummy class for pandas.Series."""
    def __init__(self, *args, **kwargs):
        pass

[jmoralez]

I like the refactor because of the following reasons:

• Moving the collection definition inside the function instead of having that big parametrize seems cleaner.
• Now the test will run for lists and numpy arrays when pandas isn't installed, whereas previously the whole test was (supposed to be) skipped.
• In case the test fails the message is more descriptive, i.e. FAILED tests/python_package_test/test_basic.py::test_list_to_1d_numpy[float32-2d_np] vs FAILED tests/python_package_test/test_basic.py::test_list_to_1d_numpy[float64-y1].

class pd_Series:  # type: ignore
    """Dummy class for pandas.Series."""
    def __init__(self, *args, **kwargs):
        pass

@strobelTha would you like to contribute those changes?

[StrikerRUS]

@jmoralez I liked reasons #2 and #3 you listed above. Thanks a lot for emphasizing advantages of the current refactor.

And indeed, we can add constructors for our dummy classes in a separate PR.

[StrikerRUS]

LGTM!
Refer to #4639 (comment) for my more detailed comment.

[github-actions]

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.