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

[BUG] Accepting prereleases as valid python version #389

Merged
merged 11 commits into from
Dec 29, 2024

Conversation

Abelarm
Copy link
Contributor

@Abelarm Abelarm commented Dec 18, 2024

Reference Issues/PRs

Fixes: sktime/sktime#7517

Make "rc" python version recognizable during python version checking.
Introducing a parameter prereleases=True
The following code snipped now returns true:

"3.11.0rc1" in SpecifierSet(">3.8", prereleases=True) 

Does your contribution introduce a new dependency? If yes, which one?

No

What should a reviewer concentrate their feedback on?

On the tests regarding checking python versions.

PR checklist

For all contributions
  • I've reviewed the project documentation on contributing
  • I've added myself to the list of contributors.
  • The PR title starts with either [ENH], [CI/CD], [MNT], [DOC], or [BUG] indicating whether
    the PR topic is related to enhancement, CI/CD, maintenance, documentation, or a bug.
For code contributions
  • Unit tests have been added covering code functionality
  • Appropriate docstrings have been added (see documentation standards)

@Abelarm Abelarm changed the title Added parameter for working with 'rc' versions [BUG] Accepting prereleases as valid python version Dec 18, 2024
@fkiraly fkiraly added implementing framework Implementing core skbase framework enhancement Adding new functionality bug Something isn't working and removed enhancement Adding new functionality labels Dec 19, 2024
Copy link

codecov bot commented Dec 19, 2024

Codecov Report

Attention: Patch coverage is 95.65217% with 1 line in your changes missing coverage. Please review.

Project coverage is 85.77%. Comparing base (306958d) to head (b0359c2).
Report is 93 commits behind head on main.

Files with missing lines Patch % Lines
...tils/dependencies/tests/test_check_dependencies.py 94.73% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #389      +/-   ##
==========================================
+ Coverage   85.07%   85.77%   +0.70%     
==========================================
  Files          45       48       +3     
  Lines        3015     3311     +296     
==========================================
+ Hits         2565     2840     +275     
- Misses        450      471      +21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

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

Thanks!

Made minor update to the docstring for clarity.

@Abelarm
Copy link
Contributor Author

Abelarm commented Dec 20, 2024

I added a super minor change to mirror the exact code of sktime. (it was just an improved error message)

Copy link
Contributor

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

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

Could you kindly add the tests here that are also present in the sktime mirror of this?

Given that we will deduplicate with this as the ultimate location.

@Abelarm
Copy link
Contributor Author

Abelarm commented Dec 21, 2024

Could you kindly add the tests here that are also present in the sktime mirror of this?

Given that we will deduplicate with this as the ultimate location.

Solved :)

@Abelarm Abelarm requested a review from fkiraly December 21, 2024 12:41
Copy link
Contributor

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

Copy link
Contributor

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

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

minor but consequential, breaking error above. Easy fix!

@fkiraly fkiraly merged commit c7a75ba into sktime:main Dec 29, 2024
23 checks passed
@Abelarm Abelarm deleted the fix_prereleases branch December 29, 2024 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working implementing framework Implementing core skbase framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] base framework does not recognize python 3.11.0rc1 as compatible with python_version tag >=3.8
2 participants