-
-
Notifications
You must be signed in to change notification settings - Fork 18k
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: Removing Python 3.6 or higher references that are always true #29492
Conversation
mypy complaint |
return "'>' not supported between instances of" in str(e) | ||
|
||
return "unorderable" in str(e) | ||
return "'>' not supported between instances of" in str(e) |
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.
and update docstring here
pandas/core/generic.py
Outdated
# We need the TYPE_CHECKING, because _attrs is not a class attribute | ||
# and Py35 doesn't support the new syntax. | ||
_attrs = {} # type: Dict[Optional[Hashable], Any] | ||
_attrs = {} # type: Dict[Hashable, Any] |
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.
I don't think the optional should be removed. This looks like an inconsistency between the code and the TODO.
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.
and the intent was to remove the TYPE_CHECKING block and use py3.6 syntax for the variable annotation
@@ -550,7 +545,7 @@ def _list_of_dict_to_arrays(data, columns, coerce_float=False, dtype=None): | |||
""" | |||
if columns is None: | |||
gen = (list(x.keys()) for x in data) | |||
types = (dict, OrderedDict) if PY36 else OrderedDict | |||
types = (dict, OrderedDict) |
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.
maybe update docstring and inline types
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.
I think its also worth inlining types now that types is a less complex expression and only used once.
This should be ready now, let me know if there is anything else. |
Co-Authored-By: Simon Hawkins <simonjayhawkins@gmail.com>
Thanks for the clarification @simonjayhawkins, I made the change. |
lgtm pending #29492 (comment) |
Sorry, I missed that comment. Instead of updating the docstring I removed that function. With py36+ only, the function was a one liner, used just once. And it was marked as private, and used in a separate file. So, I think it was better to simply get rid of it. |
not sure about this, was planning to re-use #25569 (comment) intstead of creating custom exceptions |
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.
@datapythonista Thanks for the updates. lgtm.
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.
From a quick glance, looks good to me!
@@ -550,8 +545,7 @@ def _list_of_dict_to_arrays(data, columns, coerce_float=False, dtype=None): | |||
""" | |||
if columns is None: | |||
gen = (list(x.keys()) for x in data) | |||
types = (dict, OrderedDict) if PY36 else OrderedDict | |||
sort = not any(isinstance(d, types) for d in data) | |||
sort = not any(isinstance(d, (dict, OrderedDict)) for d in data) |
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.
doesnt OrderedDict subclass dict?
@@ -26,9 +24,6 @@ | |||
from pandas.core.internals import BlockManager, SingleBlockManager, make_block | |||
import pandas.util.testing as tm | |||
|
|||
# in 3.6.1 a c-api slicing function changed, see src/compat_helper.h | |||
PY361 = LooseVersion(sys.version) >= LooseVersion("3.6.1") |
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.
have we dropped 3.6.0?
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.
our min is 3.6.1 i think
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, something is broken in 3.6.0, we were getting this test failures:
_______________ TestFrozenNDArray.test_tricky_container_to_bytes _______________
[gw0] linux -- Python 3.6.0 /home/vsts/miniconda3/envs/pandas-dev/bin/python
self = <pandas.tests.indexes.test_frozen.TestFrozenNDArray object at 0x7fd89d5b6d68>
def test_tricky_container_to_bytes(self):
> bytes(self.unicode_container)
E TypeError: only integer scalar arrays can be converted to a scalar index
pandas/tests/indexes/test_frozen.py:79: TypeError
_______________ TestFrozenNDArray.test_string_methods_dont_fail ________________
[gw0] linux -- Python 3.6.0 /home/vsts/miniconda3/envs/pandas-dev/bin/python
self = <pandas.tests.indexes.test_frozen.TestFrozenNDArray object at 0x7fd89d037e80>
def test_string_methods_dont_fail(self):
repr(self.container)
str(self.container)
> bytes(self.container)
E TypeError: only integer scalar arrays can be converted to a scalar index
pandas/tests/test_base.py:47: TypeError
So we require 3.6.1 since #29212
lgtm. can you merge master and @jbrockmendel has a comment, ping on green. |
thanks @datapythonista |
Follow up of #29212. After removing Python 3.5 compatibility, checks for >=py36 are always true and can be removed.
CC: @jreback