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

Update dependencies #382

Merged
merged 1 commit into from
Oct 2, 2018
Merged

Update dependencies #382

merged 1 commit into from
Oct 2, 2018

Conversation

linusg
Copy link
Contributor

@linusg linusg commented Oct 1, 2018

Update the listed versions of youtube_dl, mutagen, beautifulsoup4 and PyYAML to their newest versions.

IMO we should use == instead of >= in setup.py, as the former will ensure compatibility of the installed packages with spotdl while the latter would (AFAIK) not update an already existing package to its newest version, which can be important especially with packages like youtube_dl.

@vn-ki
Copy link
Contributor

vn-ki commented Oct 1, 2018

IMO we should use == instead of >= in setup.py

Packages which are released on PyPI should NOT pin their packages.

Say spotdl pins a package to a said version and another application pins the same package to another version, then we have a package conflict.

The packaging situation of Python is a mess, I agree. There are a lot of arguments in the about packaging which would be too much to include in this comment.

So I think, we should not pin our dependencies but go with the lowest version which is know to work.

PS: I'm talking from the knowledge of reading blog posts and issues over at pipenv, so what I'm saying might be a very opinionated pov.

@linusg
Copy link
Contributor Author

linusg commented Oct 1, 2018

Say spotdl pins a package to a said version and another application pins the same package to another version, then we have a package conflict.

I'd say that would be rare but I see the potential problem.

The packaging situation of Python is a mess, I agree. There are a lot of arguments in the about packaging which would be too much to include in this comment.

How would using NPM for example be different?

So I think, we should not pin our dependencies but go with the lowest version which is know to work.

That would mean at least regularly updating the version of youtube_dl whenever YouTube changes something on their side which requires a new version of the package.
And for now I'm really not sure if what would happen when the user has installed an ancient version of let's say logzero which we once "verified to work with spotdl" and has since then received various bugfix updates. If said user now installs spotdl with its setup.py containing logzero >= ancient.version.here, will this perform an update of logzero?

@vn-ki
Copy link
Contributor

vn-ki commented Oct 1, 2018

How would using NPM for example be different?

NPM has a very different ideology. Something I have a love and hate relationship with. NPM aims to give deterministic builds for each application. This means every package can have it's own pinned dependencies which won't create problems to other packages.

This won't create a problem with npm ecosystem because npm has multiple layers of dependencies (as in there can be more than one version of the same package). This is not the case with python where the dependencies are single layered. Only one version of a package is installed at a given point in time.

updating the version of youtube_dl whenever YouTube changes

This is inevitable.

If said user now installs spotdl with its setup.py containing logzero >= ancient.version.here, will this perform an update of logzero?

No. Because the requirement is already satisfied, it won't update.

@linusg
Copy link
Contributor Author

linusg commented Oct 1, 2018

Thanks for the explanations. On a side note, you just made me feel like I just started programming a few months ago 😄

No. Because the requirement is already satisfied, it won't update.

And that's why I feel at least using >= with a fairly new / the newest version in setup.py should be the way to go.

I agree == is nothing we should use publicly (I've used that in a few company internal tools, no issues so far...), what's your opinion on that?
Edit: since that's exactly what this PR is doing :)

@vn-ki
Copy link
Contributor

vn-ki commented Oct 1, 2018

I think I'm go for this PR. It has some important updates anyway. (PyYaml 3.12 won't compile with python 3.7, for example.)

@ritiek
Copy link
Member

ritiek commented Oct 2, 2018

Don't see a problem with merging this. By the way, some good discussion going on in here!

@ritiek ritiek merged commit ac94cf4 into master Oct 2, 2018
@ritiek ritiek deleted the update-dependencies branch October 2, 2018 06:51
@ritiek ritiek added this to the v1.1.0 milestone Nov 13, 2018
@ritiek ritiek mentioned this pull request Nov 13, 2018
6 tasks
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

3 participants