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

[SCSB-170] pin Python<3.13 #139

Merged
merged 1 commit into from
Aug 13, 2024
Merged

Conversation

zacharyburnett
Copy link
Collaborator

@zacharyburnett zacharyburnett commented Aug 12, 2024

Resolves SCSB-170

@zacharyburnett zacharyburnett self-assigned this Aug 12, 2024
Copy link

codecov bot commented Aug 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.95%. Comparing base (d4af8fd) to head (e50b818).
Report is 46 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #139   +/-   ##
=======================================
  Coverage   89.95%   89.95%           
=======================================
  Files          17       17           
  Lines        1792     1792           
=======================================
  Hits         1612     1612           
  Misses        180      180           

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

@zacharyburnett zacharyburnett marked this pull request as ready for review August 12, 2024 16:12
@zacharyburnett zacharyburnett requested a review from a team as a code owner August 12, 2024 16:12
@zacharyburnett zacharyburnett changed the title build wheels for Python<3.13 [SCSB-170] pin Python<3.13 Aug 12, 2024
@schlafly
Copy link
Collaborator

I'm merging this since it resolves the build failures, but I'm confused about the content of the PR, which looks more like "build wheels for python != 3.13" and the title of the PR (pin python <3.13).

@schlafly schlafly merged commit 8b70da1 into spacetelescope:main Aug 13, 2024
19 of 20 checks passed
@zacharyburnett zacharyburnett deleted the scsb170 branch August 13, 2024 12:19
@zacharyburnett
Copy link
Collaborator Author

I'm merging this since it resolves the build failures, but I'm confused about the content of the PR, which looks more like "build wheels for python != 3.13" and the title of the PR (pin python <3.13).

Oh sorry! I was working with a very similar PR in another tab and must have gotten them confused.

@schlafly
Copy link
Collaborator

Sounds good, thanks. Do you understand the issue? Somehow I expect python 3.13 problems to manifest as syntax errors or unit test errors and not only in compiling the binaries, but maybe if cython isn't happy with 3.13 then it might only appear when building wheels.

@zacharyburnett
Copy link
Collaborator Author

Sounds good, thanks. Do you understand the issue? Somehow I expect python 3.13 problems to manifest as syntax errors or unit test errors and not only in compiling the binaries, but maybe if cython isn't happy with 3.13 then it might only appear when building wheels.

I don't understand the issue, but from further experimentation with cibuildwheel yesterday I think we should revert this change and instead pin Python in pyproject.toml until we can figure out why it's failing; that will be much more maintainable and also prevent users from trying to install on 3.13 and getting installation errors.

I'll make another PR

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.

2 participants