Skip to content

[CLN] cleanup import reverse-dependencies #21844

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 1 commit into from
Jul 11, 2018

Conversation

jbrockmendel
Copy link
Member

Parts of the code are made more difficult to reason about by depending on pandas.io.formats, which has dependencies all over the code.

This PR starts in on this by replacing import Foo with import ABCFoo where possible. Along the way it cleans up import order.

@WillAyd
Copy link
Member

WillAyd commented Jul 10, 2018

Think this will have any impact on the overall import time? @TomAugspurger do you know where that original issue was?

@codecov
Copy link

codecov bot commented Jul 10, 2018

Codecov Report

Merging #21844 into master will decrease coverage by <.01%.
The diff coverage is 97.91%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #21844      +/-   ##
==========================================
- Coverage   91.92%   91.92%   -0.01%     
==========================================
  Files         160      160              
  Lines       49916    49906      -10     
==========================================
- Hits        45885    45875      -10     
  Misses       4031     4031
Flag Coverage Δ
#multiple 90.3% <97.91%> (-0.01%) ⬇️
#single 42.11% <44.79%> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/plotting/_converter.py 63.2% <100%> (ø) ⬆️
pandas/plotting/_timeseries.py 64.94% <100%> (-0.36%) ⬇️
pandas/io/clipboards.py 100% <100%> (ø) ⬆️
pandas/io/formats/csvs.py 97.66% <100%> (-0.02%) ⬇️
pandas/plotting/_tools.py 72.62% <100%> (-0.31%) ⬇️
pandas/io/formats/format.py 98.24% <100%> (-0.01%) ⬇️
pandas/io/formats/excel.py 97.37% <100%> (ø) ⬆️
pandas/io/pytables.py 92.48% <100%> (ø) ⬆️
pandas/io/formats/html.py 88.85% <100%> (ø) ⬆️
pandas/io/sas/sas_xport.py 90.27% <100%> (ø) ⬆️
... and 3 more

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 1dd05cc...7d246a3. Read the comment docs.

@jreback jreback added the Clean label Jul 11, 2018
@jreback jreback added this to the 0.24.0 milestone Jul 11, 2018
@jreback jreback merged commit 5d0daa0 into pandas-dev:master Jul 11, 2018
@jreback
Copy link
Contributor

jreback commented Jul 11, 2018

thanks @jbrockmendel

yeah this is almost cosmetic as python optimizes the imports, but it is nicer if can consolidate the imports at the top and avoid importing-chain deps.

if you can contrive a linter to actually conform the import orderings (e.g. _libs, util, compat, core), would be great, but might be tricky.

@jbrockmendel jbrockmendel deleted the iodeps branch July 11, 2018 01:19
Sup3rGeo pushed a commit to Sup3rGeo/pandas that referenced this pull request Oct 1, 2018
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.

3 participants