Skip to content

chore: document upper bound for python lib 'holidays' >= 0.26#28747

Merged
mistercrunch merged 2 commits into
masterfrom
holidays
Sep 30, 2024
Merged

chore: document upper bound for python lib 'holidays' >= 0.26#28747
mistercrunch merged 2 commits into
masterfrom
holidays

Conversation

@mistercrunch
Copy link
Copy Markdown
Member

@mistercrunch mistercrunch commented May 28, 2024

Change of plans, adding a note documenting the upper bound

I simply removed the upper bound for the holidays lib and ran pip-compile-multi -P holidays. Note that pip-compile-multi isn't as' deterministic as we'd like for it to be, but things should be fine as "# vias" are just informational here.

For the record: https://github.com/vacanza/python-holidays/blob/dev/CHANGES

@request-info request-info Bot added the requires:more-info Requires more information from author label May 28, 2024
@apache apache deleted a comment from request-info Bot May 28, 2024
Copy link
Copy Markdown
Member

@john-bodley john-bodley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michael-s-molina or @villebro do either of you know why we previously had an upper bound of 0.26 here?

Comment thread pyproject.toml Outdated
"gunicorn>=22.0.0; sys_platform != 'win32'",
"hashids>=1.3.1, <2",
"holidays>=0.25, <0.26",
"holidays>=0.25",
Copy link
Copy Markdown
Member

@john-bodley john-bodley May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we bump the lower bound and also place an upper bound of 1.0.0 as well?

@john-bodley
Copy link
Copy Markdown
Member

@mistercrunch it would be good to determine if the issue in #25017 has been resolved.

@mistercrunch
Copy link
Copy Markdown
Member Author

@john-bodley great catch! I totally would have missed that. I think we really need to add comments for all real upper bounds in pyroject.toml, including if it's just "based on semantic versioning and/or stress levels around this specific dependency".

I'm planning on implementing something like supersetbot bump-python --disregard-upper-bound which would do as specified and remove the upper bound in the process. We may need more parsable hints as smart comments so that @supersetbot knows what to do with it say:

"holidays>=0.25, <0.26", #known-upper-bound-issue #couple-with-lib=prophet

or something like

"holidays>=0.25, <1", #semver-upper-bound 

@mistercrunch mistercrunch added the hold! On hold label May 30, 2024
I simply removed the upper bound for the holidays lib and ran
`pip-compile-multi -P holidays`. Note that `pip-compile-multi`
isn't as' deterministic as we'd like for it to be, but things should be
fine as "# vias" are just informational here.
@mistercrunch mistercrunch changed the title chore: bump python 'holidays' lib from 0.25 to 0.49 chore: document upper bound for python lib 'holidays' >= 0.26 Sep 19, 2024
@mistercrunch mistercrunch removed the hold! On hold label Sep 19, 2024
@mistercrunch
Copy link
Copy Markdown
Member Author

@rusackas closing/cleaning some old PRs if you don't mind stamping this

@mistercrunch mistercrunch merged commit 9a5e8a4 into master Sep 30, 2024
@mistercrunch mistercrunch deleted the holidays branch September 30, 2024 20:23
nyohasstium pushed a commit to Webgains/superset that referenced this pull request Jan 2, 2025
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 5.0.0 First shipped in 5.0.0 labels Jul 29, 2025
qfcwell pushed a commit to qfcwell/superset that referenced this pull request May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels preset-io requires:more-info Requires more information from author size/XS 🚢 5.0.0 First shipped in 5.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants