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

Add pip listing method #36

Closed
wants to merge 7 commits into from
Closed

Add pip listing method #36

wants to merge 7 commits into from

Conversation

banesullivan
Copy link
Owner

This adds a way to create pip requirements files addressing part of #35

@banesullivan banesullivan requested a review from prisae July 25, 2019 06:26
@prisae
Copy link
Collaborator

prisae commented Jul 25, 2019

Just for clarification, how does that differ from a pip freeze? I guess only the packages you want. This, on the other hand, just revealed another way how to find version numbers... with pip freeze.

@prisae
Copy link
Collaborator

prisae commented Jul 25, 2019

E.g., the dummy package no-version, which I created and has no __version__ info, is reported by pip freeze as no-version==0.1.0; so pip freeze must have some clever features how to get version number, which might be useful for us.

@prisae
Copy link
Collaborator

prisae commented Jul 25, 2019

Maybe the whole version-finding stuff we have could be replaced with grepping the pip freeze output? It also finds vtk and pyqt without any fuss and extra loops...

@banesullivan
Copy link
Owner Author

Just for clarification, how does that differ from a pip freeze? I guess only the packages you want.

Exactly - I often work in cluttered, experimental environments and don't need to share the versions of everything for a given project.

pip freeze must have some clever features how to get version number, which might be useful for us.

That's a really interesting find... we should take a look in that source to see how they implemented it.

Maybe the whole version-finding stuff we have could be replaced with grepping the pip freeze output?

That might be a nice approach to implement

Does that work for packages installed with anaconda as well?

@prisae
Copy link
Collaborator

prisae commented Jul 25, 2019

Yes it does work for conda-installed packages too.

@prisae
Copy link
Collaborator

prisae commented Jul 25, 2019

Instead of making the user write

with open('requirements.txt', 'w') as f:
    f.write(report.listing())

couldn't you implement that directly in report.listing()? Maybe with an optional parameter, e.g.,

def listing(write=True):
   ...

so if write=True, it is written to ./requirements.txt, if it is false, it is returned to the user.

So the user would only have to do

report.listing()

to write requirements.txt.

@prisae
Copy link
Collaborator

prisae commented Jul 25, 2019

That's a really interesting find...

I do use pip freeze every now and then, but I never made the link to Versions or now Report.

@prisae prisae mentioned this pull request Jul 25, 2019
README.md Show resolved Hide resolved
scooby/report.py Outdated Show resolved Hide resolved
scooby/report.py Show resolved Hide resolved
@banesullivan
Copy link
Owner Author

I think everything is addressed now. @prisae can you approve whenever you get a chance?

@banesullivan
Copy link
Owner Author

banesullivan commented Jul 30, 2019

One incredibly annoying thing to consider - many packages out there have PyPI names that vary from their import statement, e.g. sklearn is pip install scikit-learn and import sklearn. We might have to use pip for these scenarios...

@prisae
Copy link
Collaborator

prisae commented Jul 30, 2019

One incredibly annoying thing to consider - many packages out there have PyPI names that vary from their import statement, e.g. sklearn is pip install scikit-learn and import sklearn. We might have to use pip for these scenarios...

I do not think this as an issue, because the reporting always works with the name as you would import it. You can even provide strings or packages itself. E.g.

import scooby
scooby.Report('scipy')
import scipy
scooby.Report(scipy)

So there should be no confusion, IMHO. I usually use packages, not strings, because I am interested in the things I actually imported, so directly provided them works just fine. Maybe there are other scenarios which might differ.

@banesullivan
Copy link
Owner Author

I do not think this as an issue, because the reporting always works with the name as you would import it.

I'm only referring to how it will be an issue for this PR when we send those packages to a requirements.txt file. pip install sklearn will not install scikit-learn This PR has scooby write sklearn in the requirements.txt file, not scikit-learn which is needed. There are plenty of other packages out there like this.

@prisae
Copy link
Collaborator

prisae commented Jul 30, 2019

Now I get you. Correct. In this case we would have to implement the pip freeze approach I guess...

@banesullivan
Copy link
Owner Author

I'm going to put this on hold for a while and see if anything comes of #37

@akaszynski akaszynski deleted the to-file branch July 23, 2022 15:26
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.

None yet

2 participants