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
ARROW-7950: [Python] Determine + test minimal pandas version + raise error when pandas is too old #6992
ARROW-7950: [Python] Determine + test minimal pandas version + raise error when pandas is too old #6992
Conversation
This is still WIP (depending on which pandas version we choose, we can clean up some things in the pandas-shim.pxi), but:
|
acffc4f
to
d7d5403
Compare
"installed".format(self._version) | ||
) | ||
else: | ||
return |
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.
Could we perhaps emit a warning here? I don't think that users expect their Pandas installation to be silently ignored.
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.
Yes, that's a good idea
python/pyarrow/tests/test_pandas.py
Outdated
@@ -2685,8 +2685,8 @@ class A: | |||
'a': pd.period_range('2000-01-01', periods=20), | |||
}) | |||
|
|||
expected_msg = 'Conversion failed for column a with type period' | |||
with pytest.raises(TypeError, match=expected_msg): | |||
expected_msg = 'Conversion failed for column a with type period|object' |
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.
Do you mean (period|object)
? (parentheses included)
d7d5403
to
adc5439
Compare
cc @wesm @xhochy @BryanCutler are you fine with
|
adc5439
to
a6513fa
Compare
I'm OK with this. The maintenance burden of supporting several years' worth of pandas releases seems like a lot to bear. If there are parties which are affected by this they should contribute to the maintenance (either monetarily or with their time) |
Sounds good to me. FWIW, Spark also has a minimum Pandas version set at 0.23.2. |
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.
+1
The Appveyor failure is unrelated |
Actually I'll hold off on merging this to confirm that @jorisvandenbossche has done everything that he planned |
👍 2 years ago released |
I further cleaned up the shim to remove if/else checks we no longer need, so should be ready now. |
Sweet thanks, merging now |
No description provided.