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

CLN: enforce the deprecation of exposing blocks in core.internals #58467

Merged

Conversation

natmokval
Copy link
Contributor

@natmokval natmokval commented Apr 28, 2024

xref #55355,
enforced the deprecation of create_block_manager_from_blocks

xref #55139
enforced the deprecation of exposing blocks in core.internals

@mroeschke mroeschke added Internals Related to non-user accessible pandas implementation Clean labels Apr 29, 2024
@natmokval natmokval marked this pull request as ready for review May 2, 2024 06:59
Comment on lines 658 to 659
if Version(pyarrow.__version__) == "10.0.1":
pytest.skip("skip the pyarrow version '10.0.1'")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if Version(pyarrow.__version__) == "10.0.1":
pytest.skip("skip the pyarrow version '10.0.1'")
pytest.importorskip("pyarrow", "10.0.1")

And similar below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for your help. It seems that it doesn't work when using pytest.importorskip("pyarrow", "10.0.1"), but it works properly with pytest.importorskip("pyarrow", "11.0.0")

@mroeschke mroeschke added this to the 3.0 milestone May 3, 2024
@mroeschke mroeschke merged commit 0962dcc into pandas-dev:main May 3, 2024
46 checks passed
@mroeschke
Copy link
Member

Thanks @natmokval

pmhatre1 pushed a commit to pmhatre1/pandas-pmhatre1 that referenced this pull request May 7, 2024
…ndas-dev#58467)

* CLN: enforce the deprecation of exposing blocks in core.internals

* remove the function __getattr__, and entries from __all__

* fix mypy error

* add a note to v3.0.0

* skip parquet tests for minimum Pyarrow version

* fixup test_basic

* fix pre-commit error

* skip pyarrow=10.0.1 in test_parquet.py

* fix mypy error

* fix pre-commit error

* fixup test_parquet.py

* fix pre-commit error

* fixup test_parquet.py

* replacee pytest.skip with pytest.importorskip

* skip pyarrow '10.0.1'
@jorisvandenbossche
Copy link
Member

I think we need to revert this, and first bump the DeprecationWarning to a FutureWarning. I actually don't care that much about the exact warning class, though, but I think that this is generally the correct way to deal with something we started deprecating with a DeprecationWarning. But I think we should care about keeping this around a bit longer (even though it is with a warning), because this simply means that you cannot use pandas 3.0 with pyarrow < 15, while we actually have a minimum version requirement of pyarrow 10

@jorisvandenbossche
Copy link
Member

I think that this is generally the correct way to deal with something we started deprecating with a DeprecationWarning

Which is similarly as Richard said in #50578 (comment)

@natmokval
Copy link
Contributor Author

I think we need to revert this, and first bump the DeprecationWarning to a FutureWarning. I actually don't care that much about the exact warning class, though, but I think that this is generally the correct way to deal with something we started deprecating with a DeprecationWarning. But I think we should care about keeping this around a bit longer (even though it is with a warning), because this simply means that you cannot use pandas 3.0 with pyarrow < 15, while we actually have a minimum version requirement of pyarrow 10

thanks @jorisvandenbossche, I opened PR to revert this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Internals Related to non-user accessible pandas implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants