Fix codecov testing

[alejoe91]

guys this took a lot to debug....

Another reason to advocate not to use mutable objects in functions!!! @samuelgarcia @chrishalcrow @h-mayorquin @zm711 @JoeZiminski

The problem was that the ComputeTemplates.set_params took operators=["average", "std"] as defaults, and these were propagated to the params dict without copy.
The export_reports then calls the plot_unit_templates, which asks for percentiles (which are present there) and somehow these get globally extended to the default kwargs...

So no more mutable arguments from now on!!!

[zm711]

Sounds good to me. I think Heberto started moving some of this over no? Was that PR ever merged....

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.57%. Comparing base (74abe6c) to head (bbe0c23).
Report is 265 commits behind head on main.

❗ Current head bbe0c23 differs from pull request most recent head fb7e45d. Consider uploading reports for the commit fb7e45d to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2777      +/-   ##
==========================================
- Coverage   70.88%   70.57%   -0.32%     
==========================================
  Files         285      288       +3     
  Lines       32108    32648     +540     
==========================================
+ Hits        22759    23040     +281     
- Misses       9349     9608     +259     

Flag	Coverage Δ
unittests	70.57% <100.00%> (-0.32%)	⬇️

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

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

[chrishalcrow]

Hello,
Just trying to make sure I understand (being a python newbie). A basic example which does something similar would be

def a_function(parms=['a','b']):
    a_dict = dict( parms = parms)
    return a_dict

a_dict = a_function()
a_dict['parms'] += ['c']   **
b_dict = a_function()
print(b_dict)
>>> {'parms': ['a', 'b', 'c']}

Python only evaluates parms the first time a_fuction is called. Then the line with two **s accesses parms directly and mutates it. So when we run the function again the mutated parms are taken instead of ['a', 'b'] (which we may naively expect).

I think the place in our code where we mutate must be the code after:

spikeinterface/src/spikeinterface/core/analyzer_extension_core.py

Line 468 in 256757c

if operator != "percentile":

?? Does that make sense? And do these lines set off any python-anti-pattern alarm bells?

[h-mayorquin]

@chrishalcrow you are correct.

The following link has a good heuristic to remember why this is the wrong way to go about it:
https://nedbatchelder.com/blog/202311/say_it_again_values_not_expressions.html
(people think on arguments as expressions but you are passing values!)

Regarding how to handle it, I have seen three ways to handle this:

1. Use an expression inside the code like that correction of Sam above.
2. Make local copies of all the mutable options at the beginning of the function.
3. Use non-mutable options like the Zach comment above (but difficult for other structures like dict).

In general is is a bad idea to change state or rely on the order of execution as an implicit way of defining the flow. You see the guys from scientific python defending this idea here:
https://learn.scientific-python.org/development/principles/design/#avoid-changing-state

That principle is hard to follow though : /

[chrishalcrow]

Excellent detective work Alessio! And I think I understand what's going on... 👍

[chrishalcrow]

In general is is a bad idea to change state or rely on the order of execution as an implicit way of defining the flow. You see the guys from scientific python defending this idea here:
https://learn.scientific-python.org/development/principles/design/#avoid-changing-state

Thanks for the explanation and this link -- that whole site is very informative!

[samuelgarcia]

Regarding how to handle it, I have seen three ways to handle this:

1. Use an expression inside the code like that correction of Sam above.
2. Make local copies of all the mutable options at the beginning of the function.
3. Use non-mutable options like the Zach comment above (but difficult for other structures like dict).

I prefer the version 2. over the 1.

The 3. is the best but only works with tuple that replace list for dict it is more tricky.