Skip to content
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

[REF] separate blocks.py out of internals.__init__ #22014

Merged
merged 2 commits into from
Jul 23, 2018

Conversation

jbrockmendel
Copy link
Member

Follow-up to #21903

@pep8speaks
Copy link

pep8speaks commented Jul 21, 2018

Hello @jbrockmendel! Thanks for updating the PR.

Line 666:9: E722 do not use bare except'
Line 1136:9: E722 do not use bare except'
Line 1151:9: E722 do not use bare except'
Line 2377:13: E722 do not use bare except'
Line 3057:9: E722 do not use bare except'

Comment last updated on July 21, 2018 at 22:16 Hours UTC

@codecov
Copy link

codecov bot commented Jul 21, 2018

Codecov Report

Merging #22014 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #22014      +/-   ##
==========================================
+ Coverage   91.99%   91.99%   +<.01%     
==========================================
  Files         167      168       +1     
  Lines       50578    50595      +17     
==========================================
+ Hits        46530    46547      +17     
  Misses       4048     4048
Flag Coverage Δ
#multiple 90.4% <ø> (ø) ⬆️
#single 42.19% <ø> (+0.01%) ⬆️
Impacted Files Coverage Δ
pandas/core/internals/__init__.py 96.88% <ø> (+1.38%) ⬆️
pandas/core/internals/blocks.py 94.46% <ø> (ø)

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 67b6277...aa4d7f2. Read the comment docs.

@gfyoung gfyoung added the Refactor Internal refactoring of code label Jul 23, 2018
@jreback jreback added this to the 0.24.0 milestone Jul 23, 2018

mask = mask.any(0)
new_values = new_values.T[mask]
new_placement = new_p
Copy link
Contributor

Choose a reason for hiding this comment

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

this prob doesn't belong here (move in future)


mask = mask.any(0)
new_values = new_values.T[mask]
new_placement = new_p
Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't belong here), actually a lot of things done't belong here. but I suppose that's for followups.

@jreback
Copy link
Contributor

jreback commented Jul 23, 2018

thanks, ok for now. This needs further splitting (and ultimately everything should be moved out of __init__ to separate modules with minimal dependencies on each other. But I know thats in the plan.

@jreback jreback merged commit 716efd3 into pandas-dev:master Jul 23, 2018
@@ -5,7 +5,8 @@

import pandas as pd
from pandas.core.internals import (
BlockManager, SingleBlockManager, NonConsolidatableMixIn, Block)
BlockManager, SingleBlockManager)
from pandas.core.internals.blocks import Block, NonConsolidatableMixIn
Copy link
Member

Choose a reason for hiding this comment

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

For next PRs doing further splitting, can we please keep the existing imports?

What I mean is: import all the things that are used in pandas outside of internals itself in internals/__init_.py, then no changes to imports are needed, and it is also clear which functions/classes are used in the rest of pandas.

Copy link
Member Author

Choose a reason for hiding this comment

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

For next PRs doing further splitting, can we please keep the existing imports?

Yes. I'm also marking which non-internals modules are importing what, in the hopes we can trim down the exposed surface.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Refactor Internal refactoring of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants