Skip to content

STY: absolute imports in __init__ files #30828

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 9, 2020
Merged

STY: absolute imports in __init__ files #30828

merged 1 commit into from
Jan 9, 2020

Conversation

ShaharNaveh
Copy link
Member

  • closes #xxxx
  • tests added / passed
  • passes black pandas
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

@ShaharNaveh ShaharNaveh changed the title STY: absolute imports STY: absolute imports in __init__ files Jan 8, 2020
WillAyd
WillAyd approved these changes Jan 8, 2020
View reviewed changes
Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

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

lgtm ping on green

pandas/core/arrays/__init__.py
@@ -1,16 +1,16 @@
from .base import ( # noqa: F401
from pandas.core.arrays.base import ( # noqa: F401
Copy link
Member

Choose a reason for hiding this comment

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

If you wanted to clean up more I think preferable to specify all instead of noqa: F401 for these

Copy link
Member Author

Choose a reason for hiding this comment

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

can you give me an example?
I don't understand what do you mean by all or you meant __all__?

Copy link
Member

Choose a reason for hiding this comment

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

Yea the latter

@WillAyd WillAyd added the Code Style Code style, linting, code_checks label Jan 8, 2020
@WillAyd WillAyd added this to the 1.0 milestone Jan 9, 2020
@WillAyd WillAyd merged commit ba58ead into pandas-dev:master Jan 9, 2020
@ShaharNaveh
Copy link
Member Author

I was just about to ping :)

@WillAyd
Copy link
Member

WillAyd commented Jan 9, 2020

Thanks @MomIsBestFriend

@ShaharNaveh ShaharNaveh deleted the STY-abs-import-__init__ branch January 9, 2020 00:15
@ShaharNaveh ShaharNaveh mentioned this pull request Jan 9, 2020
Merged
5 tasks
@jorisvandenbossche
Copy link
Member

jorisvandenbossche commented Jan 9, 2020 •
edited
Loading

Personally, I am -1 on those changes (or #30827).
@MomIsBestFriend before you do any more of such PRs on absolute imports (although I don't know if there are cases left?), can we first have a discussion about this?

@ShaharNaveh
Copy link
Member Author

@jorisvandenbossche I actually did want to discuss this, so I opened this PR which as you can see has only two changes. And when it got merged I saw it as an approval to go and change even more files.

Anyway, I don't mind reverting #30822, #30827, #30828.
Until this topic is fully discussed.

@ShaharNaveh ShaharNaveh mentioned this pull request Jan 17, 2020
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WillAyd WillAyd WillAyd approved these changes

Assignees
No one assigned
Labels
Code Style Code style, linting, code_checks
Projects
None yet
Milestone
1.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@ShaharNaveh @WillAyd @jorisvandenbossche