-
Notifications
You must be signed in to change notification settings - Fork 33
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
Fix deprecation and test #2021
Fix deprecation and test #2021
Conversation
@@ -28,6 +28,9 @@ | |||
|
|||
if not has_pandas(): | |||
pytest.skip("pandas>=1.0,<3.0 not installed", allow_module_level=True) | |||
else: | |||
pd = pytest.importorskip("pandas") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't make sense to me - what are we trying to do here? The original line worked correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
original code is not checking for the pandas version. New code wasn't importing pandas, even if it exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pd = pytest.importorskip("pandas") | |
import pandas as pd |
The above call to has_pandas()
+pytest.skip()
already skips the test file if pandas isn't installed (or if it's the wrong version). Thus if has_pandas()
evaluates to True
, you can simply import pandas directly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since it wasn't explicitly stated, the reason that the CI for #2016 passed is because pandas is not in pyproject.toml
. Thus it is not installed and all the tests in test_pandas_dataframe.py
are skipped
@@ -28,6 +28,9 @@ | |||
|
|||
if not has_pandas(): | |||
pytest.skip("pandas>=1.0,<3.0 not installed", allow_module_level=True) | |||
else: | |||
pd = pytest.importorskip("pandas") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pd = pytest.importorskip("pandas") | |
import pandas as pd |
The above call to has_pandas()
+pytest.skip()
already skips the test file if pandas isn't installed (or if it's the wrong version). Thus if has_pandas()
evaluates to True
, you can simply import pandas directly
@kounelisagis thanks for the quick fixes! In the future, could you please |
This PR fixes: