Fix Failing Dailies

[pauladkisson]

Dailies were failing bc os-versions and python-versions were not being propagated properly.

Now fixed -- see here: https://github.com/catalystneuro/neuroconv/actions/runs/11463322663

Daily neuroconv docker tests are failing (see #1121), but since that is a separate problem, we can solve it separately.

[h-mayorquin]

What do you think about changing the following lines to make the argument non-required for this and the other workflows:

neuroconv/.github/workflows/testing.yml

Lines 8 to 11 in 977144e

	required: true
	type: string
	default: '["3.9", "3.10", "3.11", "3.12"]'
	os-versions:

That way we only need to keep those default versions in one place.

[pauladkisson]

That way we only need to keep those default versions in one place.

Yeah, I think that's a good idea.

[h-mayorquin]

That way we only need to keep those default versions in one place.

Yeah, I think that's a good idea.

See here for more related discussion:

#1117 (comment)

Tag me when this is ready to take a look.

[pauladkisson]

@h-mayorquin, this is ready to go.

[h-mayorquin]

Where are you setting vars.ALL_PYTHON_VERSIONS? I would like to have the ability to open a PR when adding support for new versions as I would like to run the testing suit before merging.

[pauladkisson]

Where are you setting vars.ALL_PYTHON_VERSIONS? I would like to have the ability to open a PR when adding support for new versions as I would like to run the testing suit before merging.

It's in the repository variables (right next to secrets)

[h-mayorquin]

I would prefer to have them in code so we can have a workflow where changing them requires and triggers a PR.

[bendichter]

Oh yeah, that's a good point. Is there a way to put this as a variable in code?

[pauladkisson]

Oh yeah, that's a good point. Is there a way to put this as a variable in code?

As far as I could tell, the only way to include this in the code is to define it separately for each workflow. If we want all the workflows to reference a single variable, I think it has to be a github repository variable: https://github.com/catalystneuro/neuroconv/settings/variables/actions

[h-mayorquin]

We can either use the defaults to reduce the duplication (simpler and probably the way to go) or you can write the variable to a file and use bash or an action to read them.

[pauladkisson]

@h-mayorquin how about this solution?

[h-mayorquin]

Yes, I am glad that the reading from a file option worked. This LGTM.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

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

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1113      +/-   ##
==========================================
+ 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