Skip to content

Conversation

phofl
Copy link
Member

@phofl phofl commented Feb 2, 2023

  • closes #xxxx (Replace xxxx with the GitHub issue number)
  • Tests added and passed if fixing a bug or adding a new feature
  • All code checks passed.
  • Added type annotations to new arguments/methods/functions.
  • Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

@phofl phofl added the Clean label Feb 2, 2023
"use_nullable_dtypes, dtype", [(True, "Float64"), (False, "float64")]
)
@pytest.mark.parametrize("dtype_backend", ["pandas", "pyarrow"])
def test_to_numeric_use_nullable_dtypes_error(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the pytest.importorskip("pyarrow") can be removed within the test now

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah good point, thx

expected = expected.drop("c", axis=1)
tm.assert_frame_equal(result2, expected)

@pytest.mark.skipif(pa_version_under6p0, reason="minimum pyarrow not installed")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't engine already do a similar skipif?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought so as well, but it was failing locally without pyarrow

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry, actually it's only skipped for pyarrow engine but it is imported for both engines, e.g. the test will fail if fast parquet engine is used

msg = r"parquet must have string column names"
self.check_error_on_write(df, engine, ValueError, msg)

@pytest.mark.skipif(pa_version_under6p0, reason="minimum pyarrow not installed")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

@mroeschke mroeschke added this to the 2.0 milestone Feb 4, 2023
@mroeschke mroeschke merged commit c75f5af into pandas-dev:main Feb 4, 2023
@mroeschke
Copy link
Member

Thanks @phofl

@phofl phofl deleted the cln_tests branch February 4, 2023 11:30
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