-
Notifications
You must be signed in to change notification settings - Fork 229
Switch from pipenv to poetry #75
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
Conversation
f13c4ba
to
d86f697
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Berry nice 🍓
Make is probably fine, especially with the current simplicity of the file. Invoke is good when the makefile grows in complexity.
Coverage is also good. We'll see what kind of coverage is there, and we can set some reasonable threshold depending on whether things are easily testable + important enough to be tested to that extent?
Pretty cool! Apparently, its possible to use the virtual-envs created by poetry within PyCharm, so requirements don't need to be installed separately. That's great. |
9aea9ca
to
ecf7eed
Compare
9d8cb1e
to
1b253f3
Compare
2dbda85
to
fbee1a6
Compare
36af2bb
to
afb8b7b
Compare
- dropped dev dependency on rope, isort & flake - poetry doesn't support dev scripts like pipenv, so create a makefile instead - Add pytest-cov - Use tox for testing multiple python versions in CI - Update README Update ci workflow
Hi Nat~! I've experimented today using this poetry branch. I ran into another issue #91 that first caused me to think there was a problem with this branch (protoc didn't give any output except The biggest difference I've noticed using poetry is that installing the packages takes a very long time. Around 2 minutes. Apparently, this is a known issue of poetry, and a consequence of the design decision to prefer correctness over performance, and the way python packages are published does not allow for efficient dependency resolution. I did not have any trouble that I had with pipenv before, although I was not able to install the dev dependencies using poetry. I did not find an option like I'm a bit on the fence for poetry. It seems a more robust system, but the speed is really bad and we will also lose the taskrunner. Ofcourse you're working on The main benefit of poetry seems more correct dependency resolution, but as far as I know, we don't have problems with dependencies not resolving correctly, at the moment. I admittedly had trouble starting up with pipenv, but it was mainly because I didn't know the correct commands. Updating the docs could help there. Another issue we had was adding packages with pipenv without upgrading everything else. I believe that pinning the versions in the Pipfile could be a solution for that. So in conclusions, I'm not sure we have strong arguments in favor of poetry, especially taking the downside of speed into consideration. If we can find a way to make it "fast" (30 seconds? still slower than pip), I would go along and switch to poetry. There are some tricks like generating requirements.txt and installing it with pip, but in my experiment it gave some errors while installing that. |
@boukeversteegh The main motivation for this was to make the project more approachable for new contributors. The anecdata shows that pipenv is a little bit of an obstacle for newcomers to the project, and although documentation could help, poetry is still generally easier to pick up and work with. I've never really noticed speed being an issue. My understanding is the poetry only takes the slow path when it finds ambiguities in the dependency graph. That said running I'm not sure comparing to pip (which is still supported from the poetry style pyproject.toml) is the most meaningful comparison, since I don't see why it would be a common part of a development workflow, and of course it takes half the time (16s for me), because it's only doing half the work by ignoring dev dependencies. There's no To be clear using poetry vs good old fashion setup_tools has no impact on the installation speed of the resulting wheel or egg for the end user since poetry actually uses setup_tools for this process. There's still the issue of having a handy task runner via the pipfile. I went with a makefile as a simple, familiar (sort of?) alternative solution. However since I started another nifty poetry oriented solution has popped up that can already do everything that pipfile scripts can do and more. Maybe we should just use that? |
So contributors dont have to remember to run poetry install with `-E compiler`
```bash | ||
pipenv run black . | ||
```sh | ||
make format |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe lets add the alternative poetry run black .
, because we will ditch make
pretty soon and it's not a tool most python developers will have
I believe I've faithfully reproduced the same project config with the exception of:
Open question: