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

Add python 3.13 to CI #1117

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add python 3.13 to CI #1117

wants to merge 4 commits into from

Conversation

bendichter
Copy link
Contributor

No description provided.

@h-mayorquin
Copy link
Collaborator

You need to modify the Python version here:

with: # Ternary operator: condition && value_if_true || value_if_false
python-versions: ${{ github.event.pull_request.draft == true && '["3.9"]' || '["3.9", "3.10", "3.11", "3.12"]' }}
os-versions: ${{ github.event.pull_request.draft == true && '["ubuntu-latest"]' || '["ubuntu-latest", "macos-latest", "macos-13", "windows-latest"]' }}

Maybe it would be a good idea to manage that with the defaults or remove the defaults as in the current state is confusing, what do you think? they are required but they have a default (?!).

@h-mayorquin h-mayorquin mentioned this pull request Oct 22, 2024
@pauladkisson
Copy link
Member

After #1113, adding python 3.13 shouldn't require a PR, so we can close this.

@h-mayorquin
Copy link
Collaborator

I think that adding support for new versions and old versions should still require a PR independent of how we define the versions. Both might require changes on requirements and/or code and it feels to me that a PR is a good place to discuss/highlight this.

@pauladkisson
Copy link
Member

I think that adding support for new versions and old versions should still require a PR independent of how we define the versions. Both might require changes on requirements and/or code and it feels to me that a PR is a good place to discuss/highlight this.

Well, the pyproject.toml would still need to add/remove the changed versions, so that would require a PR.

Unfortunately, I don't think there's any good way to require a PR to actually change the configuration variable for the repo.

@pauladkisson
Copy link
Member

Unfortunately, I don't think there's any good way to require a PR to actually change the configuration variable for the repo.

Well I was dead wrong there lol.

Copy link

codecov bot commented Oct 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.59%. Comparing base (36464df) to head (bf690ec).
Report is 24 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1117      +/-   ##
==========================================
+ Coverage   90.44%   90.59%   +0.15%     
==========================================
  Files         129      129              
  Lines        8055     8187     +132     
==========================================
+ Hits         7285     7417     +132     
  Misses        770      770              
Flag Coverage Δ
unittests 90.59% <ø> (+0.15%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 2 files with indirect coverage changes

@h-mayorquin
Copy link
Collaborator

h-mayorquin commented Oct 24, 2024

On the ephys side I am following here:
SpikeInterface/spikeinterface#3481
We should do the same for the ophys side

Then, what would remain at our side is some behavior I think.

@pauladkisson
Copy link
Member

On the ephys side I am following here: SpikeInterface/spikeinterface#348 We should do the same for the ophys side

Then, what would remain at our side is some behavior I think.

I'm sorry, what are you talking about?

@h-mayorquin
Copy link
Collaborator

On the ephys side I am following here: SpikeInterface/spikeinterface#348 We should do the same for the ophys side
Then, what would remain at our side is some behavior I think.

I'm sorry, what are you talking about?

Adding Python 3.13 support? : /

@pauladkisson
Copy link
Member

Based on install errors, looks like we are waiting on numba: numba/numba#9682

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.

3 participants