Use of mutable default arguments

[olaurino]

There are places in the code (usually rather old code) where mutable objects are assigned to default arguments (see for instance https://github.com/sherpa/sherpa/pull/87#issuecomment-133427022). While this is a clear mistake and should be fixed, we are deferring this fix because:

• The objects are used read-only. Potential issues may be inadvertently introduced by users, but:
• The impacted methods are not part of the UI, and they are called by other Sherpa calls, in most cases without relying on the default arguments.

In some cases the simple fix that has been applied to, e.g., PR #87, would introduce even less idiomatic python code, as the original source was not idiomatic to begin with. In these cases, a proper fix would go beyond just removing the mutable objects, and should deal with the issues in the original code.

As this does not seem to be a priority, we are going to re-evaluate this issue after the 4.8 CIAO release.

[DougBurke]

One way to check is with pylint, as I noticed this particular error today (the line numbers won't make sense as this was with a PR I'm developing, but it's more to point out the error message):

% pylint sherpa/ui/utils.py sherpa/astro/ui/utils.py -d all -e W0102
************* Module sherpa.ui.utils
sherpa/ui/utils.py:177:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
sherpa/ui/utils.py:5254:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
sherpa/ui/utils.py:7056:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module sherpa.astro.ui.utils
sherpa/astro/ui/utils.py:10006:4: W0102: Dangerous default value {} as argument (dangerous-default-value)

and this warning does look like a good match to this issue: https://pylint.pycqa.org/en/latest/user_guide/messages/warning/dangerous-default-value.html

[DougBurke]

The current problems I see are

% pylint sherpa/ -d all -e W0102
************* Module sherpa.instrument
sherpa/instrument.py:80:4: W0102: Dangerous default value [] as argument (dangerous-default-value)
sherpa/instrument.py:80:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
sherpa/instrument.py:249:4: W0102: Dangerous default value [] as argument (dangerous-default-value)
sherpa/instrument.py:249:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
sherpa/instrument.py:330:4: W0102: Dangerous default value [] as argument (dangerous-default-value)
sherpa/instrument.py:330:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module sherpa.logposterior
sherpa/logposterior.py:46:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
sherpa/logposterior.py:46:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module sherpa.astro.background
sherpa/astro/background.py:76:0: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module sherpa.astro.fake
sherpa/astro/fake.py:32:0: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module sherpa.astro.ui.utils
sherpa/astro/ui/utils.py:9969:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module sherpa.optmethods
sherpa/optmethods/__init__.py:195:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module sherpa.optmethods.ncoresnm
sherpa/optmethods/ncoresnm.py:221:4: W0102: Dangerous default value [] as argument (dangerous-default-value)
sherpa/optmethods/ncoresnm.py:241:4: W0102: Dangerous default value [] as argument (dangerous-default-value)
sherpa/optmethods/ncoresnm.py:364:4: W0102: Dangerous default value [] as argument (dangerous-default-value)
sherpa/optmethods/ncoresnm.py:410:4: W0102: Dangerous default value [] as argument (dangerous-default-value)
************* Module sherpa.estmethods
sherpa/estmethods/__init__.py:156:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
sherpa/estmethods/__init__.py:210:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
sherpa/estmethods/__init__.py:276:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module sherpa.ui.utils
sherpa/ui/utils.py:116:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
sherpa/ui/utils.py:5109:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
sherpa/ui/utils.py:6908:4: W0102: Dangerous default value {} as argument (dangerous-default-value)

-----------------------------------
Your code has been rated at 9.99/10

[olaurino]

Your code has been rated at 9.99/10

🎉

[DougBurke]

How about

% pylint --disable all sherpa

-------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 9.99/10, +0.01)

Unfortunately I can't make it go to 11.

[hamogu]

Sounds like a PR to pylint is in order ;-)