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 10 commits into
base: main
Choose a base branch
from
Open

Add python 3.13 to CI #1117

wants to merge 10 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.

@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

@h-mayorquin
Copy link
Collaborator

I removed the numpy ceiling. After #1204 I think this should work, fingers crossed. If it does not, then I will remove the numpy ceiling in a separate PR.

@h-mayorquin
Copy link
Collaborator

OK, tests failing. Let's remove the numpy ceiling on another PR:

#1206

Copy link

codecov bot commented Mar 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.39%. Comparing base (73252d8) to head (b8b726a).
Report is 3 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1117   +/-   ##
=======================================
  Coverage   90.39%   90.39%           
=======================================
  Files         133      133           
  Lines        8503     8508    +5     
=======================================
+ Hits         7686     7691    +5     
  Misses        817      817           
Flag Coverage Δ
unittests 90.39% <ø> (+<0.01%) ⬆️

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

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