darvid / hydrogen

Lightweight pip wrapper with bower support and an opinionated, npm-like workflow.
http://darvid.github.io/projects/hydrogen/
BSD 2-Clause "Simplified" License
3 stars 1 forks source link

Issue creating requirements.yml from pip freeze #3

Closed bbenne10 closed 8 years ago

bbenne10 commented 8 years ago

I know I'm making a bunch of noise about a project you've probably not thought about in quite a while, and if you would prefer, I'll be happy to fork this and maintain that, but...

In current HEAD, there exists a problem with creating the default requirements.yml from output of pip freeze, rather than from reading in a requirements.txt. I intend to fix the issue, but I wanted to open this issue to track it.

The setup: Create an empty directory and cd to it. Initialize an empty virtualenv/pyenv. Run hydrogen install --save pylama.

The problem: GroupedRequirements.load_pip_requirements calls an instance of Requirements.load using the output of pip freeze as the filename parameter. This fails because Requirements.load attempts to open this parameter.

The fix: Lolidunno. I'm still learning the code, so I'm not sure what the easiest way to fix this is. Obviously , we need the pip freeze output to instantiate a number of requirements, but I haven't dug deep enough to figure out where that happens yet (or if I need to create a function that does it explicitly for this purpose)

darvid commented 8 years ago

Not a problem, although I will add you as a collaborator so you can merge your own PRs or create branches as you like.

Briefly looking at the code, I believe I expected cmd.std_out to be a file-like object, although even if it were, it looks like Requirements.load expects requirements_file.open to be a context manager, which it is for pathlib.Path objects but obviously not for anything else.

I think the easiest fix would be to add a Requirements.loads method that just takes the contents of pip freeze or a requirements.txt file, and call that instead of load in load_pip_requirements.

bbenne10 commented 8 years ago

PR incoming for this shortly.