-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
DOC: don't use single letter variable name in _compute_forward #8727
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
Conversation
|
Good call, it makes debugging more fun. I guess variables like B can be left as is no? |
mne/forward/_compute_forward.py
Outdated
| pp = _bem_inf_fields(rr[start:stop], rmags, cosmags) | ||
| pp *= ws | ||
| pp.shape = (3 * (stop - start), -1) | ||
| pc[3 * start:3 * stop] = [bincount(bins, this_pp, bins[-1] + 1) for this_pp in pp] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pep8 to fix then +1 for MRG
|
FYI for next time, in the debugger you can type |
no that didn't work actually |
strange, I get this: |
|
yeah what you say makes sense. But anyhow, single letter variable names are not good for grepping either ... i've fixed the pep8 issue |
|
thx @jasmainak |
…ools#8727) * ENH: don't use single letter variable name * Pep8
* upstream/master: (66 commits) MRG, ENH: Add infant template downloader (mne-tools#8738) ENH: add reader for NeuroElectrics .nedf files (mne-tools#8734) DOC: improve glossary entry about fiducials (mne-tools#8763) MRG, ENH: Add Report.add_custom_css (mne-tools#8762) BUG, DOC: read_raw_egi didn't support pathlib.Path; update read_raw() docstring (mne-tools#8759) Add "dbs" as new channel type (mne-tools#8739) MRG, VIZ: Fix title position in plot_sensors (mne-tools#8752) MRG: Support for non-FIFF files in Report.parse_folder (mne-tools#8744) MRG, VIZ, FIX: sEEG picking in _prepare_topomap_plot() (mne-tools#8736) DOC: don't use single letter variable name in _compute_forward (mne-tools#8727) WIP: Fix search [skip github] [skip azp] (mne-tools#8742) WIP: Compare Beer-lambert to HOMER (mne-tools#8711) MRG: bump spyder version (mne-tools#8020) FIX anon with IO round trip (mne-tools#8731) fix set_bipolar_reference for Epochs (mne-tools#8728) WIP: Add width argument, reduce default (mne-tools#8725) ENH: Add toggle-all button to Report (mne-tools#8723) fix int/float conversion in nicolet header (mne-tools#8712) MRG, BUG: Fix Report.add_bem_to_section n_jobs != 1 (mne-tools#8713) MRG, DOC: Make "rank" options in docs more accessible (mne-tools#8707) ...
I got a very cryptic error today when computing the forward:
Not sure why this happens. When I tried debugging I realized that I couldn't look at the variables because
pconflicted with thepcommand. Hence, this pull request. Also, the error magically disappeared when I made the change and now going back to master, I can't reproduce it. Don't ask ...I think removing the single letter variable name is a good idea in any case.