Skip to content

CLN: Misc PY2/3 compat functions #26008

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

Merged
merged 10 commits into from
Apr 9, 2019
Merged

Conversation

mroeschke
Copy link
Member

  • xref CLN: remove PY2 #25725

  • tests added / passed

  • passes git diff upstream/master -u -- "*.py" | flake8 --diff

  • Import from pandas.compat.chainmap import DeepChainMap directly where used

  • Removed isidentifier, str_to_bytes, bytes_to_str, bind_method

@mroeschke mroeschke added this to the 0.25.0 milestone Apr 6, 2019
get_option('display.encoding'))
)
text = text.decode(kwargs.get('encoding') or
get_option('display.encoding'))
except AttributeError:
Copy link
Member

@jbrockmendel jbrockmendel Apr 6, 2019

Choose a reason for hiding this comment

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

Maybe take the getoption outside of the try/except?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm...that encoding logic is specific to the try-except block. I'm not sure we should be putting it outside.

In addition, since this is supposed to be pure removal, I'm uneasy about making other changes beyond the stated purpose of removing PY2 / 3 things.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fine by me. Moved back in the try except.

@@ -654,7 +654,7 @@ def test_decode_big_escape(self):
# Make sure no Exception is raised.
for _ in range(10):
base = '\u00e5'.encode("utf-8")
quote = compat.str_to_bytes("\"")
quote = "\"".encode('ascii')
Copy link
Member

Choose a reason for hiding this comment

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

b’”’

@codecov
Copy link

codecov bot commented Apr 6, 2019

Codecov Report

Merging #26008 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #26008      +/-   ##
==========================================
+ Coverage   91.82%   91.83%   +<.01%     
==========================================
  Files         175      175              
  Lines       52551    52531      -20     
==========================================
- Hits        48256    48240      -16     
+ Misses       4295     4291       -4
Flag Coverage Δ
#multiple 90.39% <100%> (+0.01%) ⬆️
#single 40.72% <66.66%> (-0.14%) ⬇️
Impacted Files Coverage Δ
pandas/io/clipboards.py 100% <ø> (ø) ⬆️
pandas/compat/__init__.py 79.2% <100%> (+3.37%) ⬆️
pandas/core/ops.py 91.72% <100%> (-0.02%) ⬇️
pandas/core/computation/scope.py 93.13% <100%> (ø) ⬆️
pandas/core/computation/pytables.py 90.24% <100%> (ø) ⬆️
pandas/io/common.py 91.87% <100%> (ø) ⬆️
pandas/core/generic.py 93.54% <100%> (ø) ⬆️
pandas/io/gbq.py 75% <0%> (-12.5%) ⬇️
pandas/core/frame.py 96.79% <0%> (-0.12%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a1fee91...598f415. Read the comment docs.

@codecov
Copy link

codecov bot commented Apr 6, 2019

Codecov Report

Merging #26008 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #26008      +/-   ##
==========================================
+ Coverage   91.82%   91.83%   +<.01%     
==========================================
  Files         175      175              
  Lines       52540    52517      -23     
==========================================
- Hits        48247    48228      -19     
+ Misses       4293     4289       -4
Flag Coverage Δ
#multiple 90.39% <100%> (+0.01%) ⬆️
#single 40.72% <66.66%> (-0.14%) ⬇️
Impacted Files Coverage Δ
pandas/io/clipboards.py 100% <ø> (ø) ⬆️
pandas/compat/__init__.py 77.9% <100%> (+3.37%) ⬆️
pandas/core/ops.py 91.72% <100%> (-0.02%) ⬇️
pandas/core/computation/scope.py 93.13% <100%> (ø) ⬆️
pandas/core/computation/pytables.py 90.24% <100%> (ø) ⬆️
pandas/io/common.py 91.83% <100%> (-0.05%) ⬇️
pandas/core/generic.py 93.54% <100%> (ø) ⬆️
pandas/io/gbq.py 75% <0%> (-12.5%) ⬇️
pandas/core/frame.py 96.79% <0%> (-0.12%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0610a60...82a353f. Read the comment docs.

@jreback
Copy link
Contributor

jreback commented Apr 7, 2019

can you merge master

@@ -27,7 +26,7 @@ def test_repr_binary_type():
raw = bytes(letters, encoding=cf.get_option('display.encoding'))
except TypeError:
raw = bytes(letters)
b = str(compat.bytes_to_str(raw))
b = str(raw.decode('utf-8'))
Copy link
Member

Choose a reason for hiding this comment

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

not a big deal, but is the str still necessary?

Copy link
Member

@gfyoung gfyoung Apr 8, 2019

Choose a reason for hiding this comment

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

Probably not...should test in a separate commit nonetheless.

@@ -654,7 +654,7 @@ def test_decode_big_escape(self):
# Make sure no Exception is raised.
for _ in range(10):
base = '\u00e5'.encode("utf-8")
quote = compat.str_to_bytes("\"")
quote = b"\""
Copy link
Member

Choose a reason for hiding this comment

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

would b'"' be clearer by avoiding the backslash?

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 think so.

@jbrockmendel
Copy link
Member

@jreback rebased+green

@mroeschke
Copy link
Member Author

I think these are flaky tests. They pass locally for me and don't relate to changes this PR makes.

_ TestCategoricalSeriesAnalytics.test_drop_duplicates_categorical_non_bool[True-datetime64[D]] _
[gw1] linux -- Python 3.6.6 /home/travis/miniconda3/envs/pandas-dev/bin/python
[XPASS(strict)] GH#7996
_ TestCategoricalSeriesAnalytics.test_drop_duplicates_categorical_non_bool[False-datetime64[D]] _
[gw1] linux -- Python 3.6.6 /home/travis/miniconda3/envs/pandas-dev/bin/python
[XPASS(strict)] GH#7996

@jreback
Copy link
Contributor

jreback commented Apr 8, 2019

yeah have seen these and can’t repro either

@jreback jreback merged commit af0ecbe into pandas-dev:master Apr 9, 2019
@jreback
Copy link
Contributor

jreback commented Apr 9, 2019

thanks @mroeschke

@mroeschke mroeschke deleted the py3_more_misc branch April 9, 2019 17:08
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.

4 participants