8000 Fix `site-packages/src` installation directory · Issue #14 · amalfra/docker-hub · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix site-packages/src installation directory #14

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
Closed
sodul opened this issue Jul 19, 2021 · 5 comments · Fixed by #15
Closed

Fix site-packages/src installation directory #14

sodul opened this issue Jul 19, 2021 · 5 comments · Fixed by #15
Assignees
@amalfra
Labels
bug enhancement

Comments

@sodul
Copy link
Contributor
sodul commented Jul 19, 2021

Thanks for providing this tool. I was looking at the possibility of using it as a library for our own python projects and I noticed that it does not get pip installed cleanly.

The src directory get installed in site-packages as src which means that if someone else does the same thing the directories will clash.

I strongly recommend that you rename the src directory to docker_hub and update the referenced accordingly.

Today we can do:

import src.libs.docker_hub_client

And being able to do:

import docker_hub.libs.docker_hub_client

would be much cleaner.

I can send a PR if you are interested. The changes should be minimal.
@amalfra amalfra self-assigned this Jul 20, 2021
@amalfra
Copy link
Owner
amalfra commented Jul 20, 2021

Thanks for pointing this out. Excited to know that more people are finding this useful.

I will have a look at the issue this weekend. Meanwhile if you have time to send a PR to fix it that would be really nice!

sodul added a commit to sodul/docker-hub that referenced this issue Jul 20, 2021
@sodul
Rename internal 'src' directory to 'docker_hub' to so `import docker_…
74d166a 
…hub` can be done.

Fixes amalfra#14
@sodul sodul mentioned this issue Jul 20, 2021
Merged
@sodul
Copy link
Contributor Author
sodul commented Jul 20, 2021

Not directly related but I noticed at least one bug in your code that mypy detected for me (utils.py line 56, you have print_header(heading), which is True but I think it should be header).

There are some additional improvements that I may suggest:

  • add an auto formatter. I recommend gray that can call black, iformat and unify for you. This will ensure your code will be more consistent. You can even add support for pre-commit where it will run on your commits to ensure the code is always consistent.
  • add pylint and mypy to discover potential bugs.
  • Drop support for python 2, and any version prior to 3.6. This will allow you to take advantage of python 3 features and syntax without having to jumps through hoops.

@sodul
Copy link
Contributor Author
sodul commented Jul 20, 2021

On that topic the checks are failing with python 2.7: https://github.com/sodul/docker-hub/runs/3118103727?check_suite_focus=true

@amalfra
Copy link
Owner
amalfra commented Jul 21, 2021

The python 2 support dropping is actually planned in the next major release v3. Probably I can start working on that soon.

Regarding the print_header(heading) part I am not quite sure if I understood the bug exactly but the print_result function is supposed to support custom headers or by default will use a header having a summary of result counts.
The autoformatting would definitely be helpful. Also as you suggested, the current pycodestyle could maybe be replaced with pylint that has static code analysis also.

Thanks, a lot for the suggestions and these are definitely something that I would add to the backlog and I am planning to have python v2 drop also released with your changes.

@roelarents
Copy link

thx. i'm using this in a python script and i still/again have to import src.libs.docker_hub_client. was it reverted?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@amalfra amalfra

Labels
bug enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Rename internal 'src' directory to 'docker_hub' sodul/docker-hub
3 participants
@sodul @roelarents @amalfra
0