Skip to content

DEPS: Add warning if pyarrow is not installed #56896

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.

Sign up for GitHub

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

Merged
merged 14 commits into from
Jan 19, 2024
Merged

DEPS: Add warning if pyarrow is not installed #56896

Changes from 6 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
bb520d1
add warning
lithomas1 Jan 15, 2024
a3b3dc3
DEPS: Add warning if pyarrow is not installed
lithomas1 Jan 15, 2024
944c288
formatting
lithomas1 Jan 15, 2024
613eac7
Update __init__.py
lithomas1 Jan 16, 2024
21509c5
adjustments
lithomas1 Jan 17, 2024
c6ae03d
adjustments
lithomas1 Jan 17, 2024
7b48501
updates
lithomas1 Jan 17, 2024
199fb7c
address code review
lithomas1 Jan 17, 2024
4637a96
Update __init__.py
lithomas1 Jan 17, 2024
aa48e19
add pyarrow to npdev build
lithomas1 Jan 19, 2024
9999fe3
Merge branch 'add-pyarrow-warning' of github.com:lithomas1/pandas int…
lithomas1 Jan 19, 2024
3aa3ba9
ignore non numpy related deprecationwarnings/futurewarning
lithomas1 Jan 19, 2024
0074b03
ignore non numpy related deprecationwarnings/futurewarning
lithomas1 Jan 19, 2024
cc2debf
Update actions-311-numpydev.yaml
lithomas1 Jan 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 33 additions & 2 deletions pandas/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,39 @@
FutureWarning,
stacklevel=2,
)
# Don't allow users to use pandas.os or pandas.warnings
del os, warnings
# Don't allow users to use pandas.os
del os
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason to not run this this at the end with the other modules? Seems like it's easier to maintain if there is all the clean up at the end or next to the import, but no big deal

Copy link
Member Author

Choose a reason for hiding this comment

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

No, not really.

Personally I like to keep the del closed to where it's being used, but no strong preference.

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 os is not being used in the code before beibg closed. As said, not important, but to me personally it makes sense to open and close around the block where it is being used, or open and close at the beginning and the end if it's an import that is used in several places. Now os is being opened at the beginning, and closed in the middle, in a kind of random place if I'm not wrong. No huge difference, but I think it'll help maintain the code if closed at the end (or we move the import and del around where it is being used).

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 put the os delete with the deletion of warnings/pa_version_under10p1.


# DeprecationWarning for missing pyarrow
from pandas.compat.pyarrow import pa_version_under10p1

if pa_version_under10p1:
# Check if old pyarrow is installed
from pandas.compat._optional import VERSIONS

try:
import pyarrow # noqa: F401
Copy link
Member

Choose a reason for hiding this comment

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

Should we add an else block to del pyarrow so pandas.pyarrow is not created?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thx


pa_msg = (
f"was too old on your system - pyarrow {VERSIONS['pyarrow']} "
"is the current minimum supported version as of this release."
)
except ImportError:
pa_msg = "was not found to be installed on your system."

warnings.warn(
f"""
Pyarrow will become a required dependency of pandas in the next major release of pandas,
Copy link
Member

@jorisvandenbossche jorisvandenbossche Jan 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
Pyarrow will become a required dependency of pandas in the next major release of pandas,
Pyarrow will become a required dependency of pandas in the next major release (pandas 3.0),

I think we can be explicit about what this next major release will be

Copy link
Contributor

@bashtage bashtage Jan 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Is the next major release also the next minor release, i.e., 2.2 then 3.0? I noticed that the dev wheels recently switched to reporting 3.0.0.dev0 recently.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is the next major release also the next minor release, i.e., 2.2 then 3.0? I noticed that the dev wheels recently switched to reporting 3.0.0.dev0 recently.

No, 2.2 is coming out sometime this week.
But yes, 3.0.0 will be the release after that.

(to allow more performant data types and better interoperability with other libraries)
Copy link
Member

Choose a reason for hiding this comment

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

I would maybe explicitly mention the new default string dtype, since that's still the only one that is used by default that requires 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.

Updated.

but {pa_msg}
If this would cause problems for you,
please provide us feedback at https://github.com/pandas-dev/pandas/issues/54466
""",
DeprecationWarning,
stacklevel=2,
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to make this stacklevel=1?

Because this code is top-level and not inside a function or method, it one less than the typical stacklevel of 2 we use in a direct function/method.
Setting it as 1 would avoid that it triggers a warning when you call a function that has an inline import pandas (not sure how important this is, though)

Copy link
Member Author

Choose a reason for hiding this comment

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

The warning doesn't pop up in the REPL when I import pandas if I do stacklevel=1.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, OK forget this. I did test it, but should have done something wrong because now I can't reproduce getting the warning with plain import and stacklevel 1

)
del VERSIONS
del pa_version_under10p1, warnings

# module level doc-string
__doc__ = """
Expand Down