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

Remove incorrect upper bound from pytz dependency #225

Closed
wants to merge 1 commit into from

Conversation

mgorny
Copy link
Contributor

@mgorny mgorny commented Mar 31, 2023

Change the pytz dependency to use the >= operator rather than ^, in order to allow installing newer versions. pytz uses calendar versioning rather than semantic versioning, so the ^ operator is meaningless there. Furthermore, since pytz version corresponds to the timezone data release, allowing the newest version is important to permit using up-to-date tzinfo.

@TkTech TkTech added this to the 3.1.0 milestone Apr 3, 2023
@TkTech TkTech changed the base branch from v3.0.2 to v3.1.0 April 3, 2023 23:12
Change the `pytz` dependency to use the `>=` operator rather than `^`,
in order to allow installing newer versions.  `pytz` uses calendar
versioning rather than semantic versioning, so the `^` operator is
meaningless there.  Furthermore, since `pytz` version corresponds
to the timezone data release, allowing the newest version is important
to permit using up-to-date tzinfo.
@mgorny
Copy link
Contributor Author

mgorny commented Apr 4, 2023

Hmm, I don't seem to be able to reproduce these test failures.

@mgorny
Copy link
Contributor Author

mgorny commented Apr 4, 2023

Hmm, rebasing seems to have helped (or rerunning).

@TkTech
Copy link
Contributor

TkTech commented Apr 4, 2023

There was another PR included in v3.1.0 that fixed the tests. The latest version of babel changed format strings to use a non-breaking-space before PM/AM. So the test failures are confusing because there's no visible difference :)

@ThiefMaster
Copy link
Contributor

A fixed release containing this PR would be great.

@ThiefMaster
Copy link
Contributor

Also, please consider not pinning any upper versions - flask-babel is a library and it should be up to the application to pin its transitive dependencies. As an application developer I do not want to wait for a library release in order to update e.g. flask, even if everything might still be compatible!

cc @davidism since you know some nice blog post links on this topic ;)

@davidism
Copy link
Contributor

davidism commented Apr 6, 2023

When writing a library, you should only use lower bounds for install dependencies. This allows people writing applications to use a tool like pip-tools to pin their full dependency tree. If libraries use restrictive bounds unnecessarily, it makes it impossible to pin to newer versions without a new release, whereas lower bounds can always be pinned more specifically in the application to work around issues.

You can review any of the following for more information about not relying on any particular semantics in version numbers, and avoiding creating version conflicts:

@TkTech
Copy link
Contributor

TkTech commented Apr 6, 2023

A PR with these changes would be accepted.

My city currently has no power and my area will likely not get power for several more days. I can accept a PR and make a release, but any code changes will have to wait until I can recharge a laptop.

@kamyar
Copy link

kamyar commented Apr 7, 2023

@TkTech Any chance you can take a look if #227 looks complete / good?

@davidism
Copy link
Contributor

davidism commented Apr 7, 2023

This was already a PR, am I missing something?

@jwag956
Copy link

jwag956 commented Apr 7, 2023

@davidism I think the confusing part is that 'master' isn't up to date with the 3.1.0 branch - so folks trying to 'test with latest' are in fact getting an older version....

@TkTech TkTech closed this Apr 7, 2023
@mgorny mgorny deleted the pytz-dep branch April 8, 2023 05:17
@mgorny
Copy link
Contributor Author

mgorny commented Apr 8, 2023

Thank you!

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.

6 participants