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

Use absolute imports everywhere #1362

Merged
merged 1 commit into from
Feb 6, 2018
Merged

Conversation

kevinkjt2000
Copy link
Contributor

No description provided.

Copy link
Collaborator

@jeffwidman jeffwidman left a comment

Choose a reason for hiding this comment

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

@kevinkjt2000 Why should we do this?

Is there a functional reason other than aesthetics?

The downside of this is that it's harder to re-organize the folder tree down the road...

.gitignore Outdated
tests-env/
.pytest_cache/
Copy link
Collaborator

Choose a reason for hiding this comment

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

These changes should probably be in a separate PR as they are unrelated to import changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed that by amending my commit.

@kevinkjt2000
Copy link
Contributor Author

@jeffwidman Python has trouble with relative imports when libraries are being imported from a zip file that is not extracted. These changes fix an issue in a new product of the company I develop for.

@dpkp
Copy link
Owner

dpkp commented Feb 1, 2018

I'm fine with this if/when tests pass.

@kevinkjt2000
Copy link
Contributor Author

The test failure seems strange to me https://travis-ci.org/dpkp/kafka-python/jobs/335863422
Looks like version 0.10.2.1 of kafka failed only with pypy.

@jeffwidman
Copy link
Collaborator

There's some flaky tests... I restarted the failing test job.

Python has trouble with relative imports when libraries are being imported from a zip file that is not extracted.

To clarify, you're saying that you are importing python code straight from a zip file without extracting it? I did not realize python let you do this...

@kevinkjt2000
Copy link
Contributor Author

@jeffwidman Yes. https://docs.python.org/2/library/zipimport.html Inserting a .zip file into the sys path allows python to import files from it. Which, after the trouble I have seen here, I would not recommend doing. All .so files also cannot load from a .zip file. Fun stuff.

@dpkp
Copy link
Owner

dpkp commented Feb 2, 2018

have you looked at https://github.com/pantsbuild/pex ?

@kevinkjt2000
Copy link
Contributor Author

@dpkp pex looks promising! I will recommend it to my co-workers that work on that codebase more often.

@dpkp dpkp merged commit 9ae0e59 into dpkp:master Feb 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants