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

Investigate pipenv #31

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

ianbrown9475
Copy link
Collaborator

I've added a simple Pipfile and Pipfile.lock for demonstration purposes. Here's what I've found so far

  • separate packages and dev-packages: Similar to npm's package.json, you can specify if packages are required for production, or if they're just intended for development. This is especially nice for automated deployments, since installation time can be cut way down if things like mypy and pytest aren't required in a production environment.
  • virtual environments:
    • pipenv will automatically create a virtualenv when pipenv install is ran. It requires a version of Python to be installed locally matching the required version in the Pipfile if one is specified.
    • It's recommended to use pyenv to manage multiple versions of python. If you have pyenv installed and the required version of python is not installed, pipenv will automatically prompt the user to install the latest matching version with pyenv. Otherwise, it will print an error saying that a matching version of python was not found on the local system.
    • You can also override the version of python to use with pipenv install --python [version or path of python executable]. This is useful if you need to test different Python versions, or if you want to run it and feel confident that you have a version of python that should work for your purposes. It will warn you if the Pipfile specifies a different version. You can also do pipenv install [--two | --three] to use the locally installed Python 2/3 executables respectively.
    • Virtualenvs appear to go in ~/.local/share/virtualenvs/[dirname]-[generated ID] by default. You can print the location of the currently installed virtualenv with pipenv --venv.
    • You can remove the currently installed virtualenv with pipenv --rm.
  • automated deployments: Helpful options for using pipenv in deployments: https://pipenv-fork.readthedocs.io/en/latest/advanced.html#using-pipenv-for-deployments. These should ideally be used in CI/CD configurations and Dockerfiles. This will allow us to enforce that the Pipfile and Pipfile.lock coincide.
  • dependency vulnerabilities: Similar to npm's npm audit, pipenv can check for vulnerabilities in existing dependencies with pipenv check. For example, here is the current output of this:
    36810: numpy <=1.16.0 resolved (1.15.1 installed)!
    An issue was discovered in NumPy 1.16.0 and earlier. It uses the pickle Python module unsafely, which allows remote attackers to execute arbitrary code via a crafted serialized object, as demonstrated by a numpy.load call.
    
  • scripts: Support for these aren't great, so these should probably stay in Makefiles, but the functionality does exist. I added a simple example that should probably be removed eventually. It actually seems like it's recommended to run pipenv from a Makefile script, instead of the other way around: https://pipenv-fork.readthedocs.io/en/latest/advanced.html#travis-ci

@ViridianForge what are your thoughts/questions? If this is all looking good, I think the next steps would be to turn this draft into an actual PR that migrates from conda + pip to pipenv.

Pipfile/Pipfile.lock generated strictly from
```
pipenv install -r requirements.txt
```
@pmolfese
Copy link

Would also encourage you to look at anaconda environments. Many of the same benefits without the drawbacks.

@ViridianForge
Copy link
Contributor

@pmolfese - What drawbacks are you thinking of?

@ViridianForge
Copy link
Contributor

@ianbrown9475 - on a topical level - I'm liking this as it brings in the only thing I was really using Anaconda for (the management of virtual environments). Frankly - I also really like the potential of automating the inclusion of any outstanding issues arising from dependencies on external packages in our regulatory documentation as we go through development.

For next stages, let's bring this up at the conversation we have planned for 2020.04.29 within the active BEL developers. If the core development team is in general agreement - we can come up with a roadmap to bringing the Python projects into alignment - and that can start with a cleanup of MFFpy in that regard.

@mscf, @damian5710, and @ephathaway, as our other primary Python developers, I encourage you to look over this and see if you have any strong feelings for or against what Ian's presenting here.

@damian5710
Copy link
Collaborator

I think this tool looks interesting.

@ianbrown9475 , do you know how stable it is? If it is stable enough maybe we could give it a chance.

@ianbrown9475
Copy link
Collaborator Author

ianbrown9475 commented Apr 28, 2020

@damian5710 the project seems to get new commits fairly often. It's been a while since the last release (November 2018), but it looks like the next release will be out within a few days/weeks (see issue 3369). I can't speak too much on the stability of the tool since I've only been using it for about 6 months, meaning I've only ever used one version of it.

There's a short section about stability here. The rest of the article has some good criticisms of pipenv, although I do have a few comments about it:

  • The main criticism of it being slow to generate a lockfile is valid. I've noticed that this can take a while when using it in the past. However, we should only need to do this when we upgrade or add/remove packages which shouldn't be too frequent. Otherwise, we'll just install packages straight from the lockfile which doesn't take that long
  • The article was written in July 2018 where, at the time, the tool seemed to be making several releases which seemed to be fairly unstable. The article was updated in February 2020 saying that the project is dead since it hasn't had a release in a while. I don't think there's much of a concern with the project being unstable at the moment, unless the team decides to start making frequent, unstable releases after the upcoming release. The project also doesn't seem dead, as commits still happen frequently and a new release should be completed soon.
  • I've also heard praise for poetry but it still seems to be fairly new.

Good call asking about the stability! I hadn't looked into this too much before but it's absolutely something we should consider.

@jusjusjus
Copy link
Collaborator

@ianbrown9475, I think you should go ahead and make the request. Before, it would be great to modify the install/build instructions in the README section. Can you also update the NumPy dependency to v1.17.1? We seem to be using that one now in many other repositories.

@ianbrown9475
Copy link
Collaborator Author

I don't think we should merge this at the moment. Since my initial look, pipenv has had some new releases and I've also heard more anecdotes about poetry being a better option, mainly due to making it easy to publish packages and having more features. I've also seen other anecdotes of individuals deciding to overhaul their organizations' repos to all use pipenv and eventually running into problems causing them to revert.

This is a big decision and we should carefully think this through and definitely compare other options like poetry. We should come back to this when we have time, maybe next sprint.

@jusjusjus
Copy link
Collaborator

Also fine with me :)

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.

5 participants