Skip to content

Ignore flake8 E741 (ambiguous variable name)#8000

Closed
hoechenberger wants to merge 1 commit intomne-tools:masterfrom
hoechenberger:flake
Closed

Ignore flake8 E741 (ambiguous variable name)#8000
hoechenberger wants to merge 1 commit intomne-tools:masterfrom
hoechenberger:flake

Conversation

@hoechenberger
Copy link
Copy Markdown
Member

@hoechenberger hoechenberger commented Jul 11, 2020 •
edited
Loading

Running flake8 on master gives me:

./tutorials/source-modeling/plot_background_freesurfer_mne.py:133:22: E741 ambiguous variable name 'I'
./tutorials/source-modeling/plot_background_freesurfer_mne.py:136:22: E741 ambiguous variable name 'I'

mne-python/tutorials/source-modeling/plot_background_freesurfer_mne.py

Lines 132 to 137 in d839a6c

ori_slice = dict(P='Coronal', A='Coronal',
I='Axial', S='Axial',
L='Sagittal', R='Saggital')
ori_names = dict(P='posterior', A='anterior',
I='inferior', S='superior',
L='left', R='right')

I don't think this rule is very useful (as we'll spot badly-named variables during the code review anyway), so this PR disables it.

@hoechenberger
Ignore flake8 E741 (ambiguous variable name)
f5a71f9 
Running flake8 on master gives me:
./tutorials/source-modeling/plot_background_freesurfer_mne.py:133:22: E741 ambiguous variable name 'I'

I don't think this rule is very useful (as we'll spot badly-named
variables during the code review anyway), so this PR disables it.
@hoechenberger
Copy link
Copy Markdown
Member Author

hoechenberger commented Jul 11, 2020 •
edited
Loading

@larsoner Travis fails with

[gw0] linux -- Python 3.6.7 /home/travis/miniconda/envs/test/bin/python3.6
4043mne/tests/test_docstring_parameters.py:161: in test_docstring_parameters
4044    raise AssertionError(msg)
4045E   AssertionError: 
4046E   mne.source_space.SourceSpaces.clear : SS03 : Summary does not end with a period
4047E   mne.source_space.SourceSpaces.reverse : SS03 : Summary does not end with a period
4048E   2 errors
4049
4050

I checked and the docstrings look alright on Py 3.7 and 3.8, so this seems to be a Py 3.6 thing…
(SourceSpaces inherits from list and we don't have our own clear and reverse implementations)

@hoechenberger hoechenberger mentioned this pull request Jul 11, 2020
Merged
@larsoner
Copy link
Copy Markdown
Member

larsoner commented Jul 12, 2020

I would rather rename the variable and keep the check

@larsoner
Copy link
Copy Markdown
Member

larsoner commented Jul 12, 2020

Or noqa it if it's a valuable exception

@hoechenberger
Copy link
Copy Markdown
Member Author

hoechenberger commented Jul 12, 2020

@larsoner
But I think that the naming that causes the messages here makes total sense in the given context (dict key), and we don't want #noqa strings in a tutorial, do we?

That said, maybe there's an easier solution:
instead of creating the dict via

 ori_slice = dict(P='Coronal', A='Coronal', 
                  I='Axial', S='Axial', 
                  L='Sagittal', R='Saggital')

we could use the {k: v} syntax, so the names flake8 complains about would become strings. And now thinking about this some more it actually seems like a flake8 bug to me in the first place!!

@hoechenberger
Copy link
Copy Markdown
Member Author

hoechenberger commented Jul 12, 2020

Seeing that the docstyle tests pass for #7991 after it has been rebased on master, I have to conclude that the flake complaint is probably something that only happens on my system -- at least it doesn't seem to happen on CI (anymore). Therefore, I'm closing this PR.

Still-- what do we do about the Travis poblem?

@hoechenberger hoechenberger deleted the flake branch July 12, 2020 07:10
@hoechenberger
Copy link
Copy Markdown
Member Author

hoechenberger commented Aug 4, 2020

Just for reference, I've updated to:

flake8-3.8.3
pyflakes-2.2.0

and I don't see this error anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@agramfort agramfort agramfort approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hoechenberger @larsoner @agramfort