Skip to content
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

Function call with wrong argument name fix #11351

Merged
merged 4 commits into from
Dec 12, 2022
Merged

Conversation

ealtamir
Copy link
Contributor

@ealtamir ealtamir commented Dec 2, 2022

What does this implement/fix?

A function call is being made with a wrong argument. This causes problems when calling the .save method with the proj=True argument on a Raw object. I changed the function call argument to follow the signature of the function.

Additional information

For easy reference, the function signature can be found at:

def make_eeg_average_ref_proj(info, activate=True, *, ch_type='eeg',

@larsoner
Copy link
Member

larsoner commented Dec 2, 2022

Can you add a test to mne/io/tests/test_proj.py that fails on main but passes on this PR (test-driven development)? Ideally it would be some version of whatever caused you to hit this bug, but if not then something like this should work:

raw = read_raw_fif(...)
raw.del_proj()
setup_proj(raw.info)

Then we just need an entry in the BUG section of doc/changes/latest.inc to get you credit for the fix!

@larsoner larsoner added this to the 1.3 milestone Dec 2, 2022
* upstream/main:
  ENH: Add webp support to Report (mne-tools#11359)
  ENH: More complete report repr (mne-tools#11357)
  MAINT: Simplify server installation instructions (mne-tools#11356)
  BUG: Fix where report replacement did not respect section (mne-tools#11346)
  [DOC] Fix video link for coregistration (mne-tools#11354)
  Revert "[ENH] Add tutorial on time-frequency source estimation with STC viewer GUI" (mne-tools#11350)
@@ -132,6 +132,8 @@

.. _Enrico Varano: https://github.com/enricovara/

.. _Enzo Altamiranda: https://www.linkedin.com/in/enzoalt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ealtamir can you confirm this is the correct name+URL for you?

@larsoner
Copy link
Member

In it goes in prep for release, but @ealtamir feel free to still comment above if the name/url needs adjustment and we'll fix it!

@larsoner larsoner merged commit ec28281 into mne-tools:main Dec 12, 2022
@welcome
Copy link

welcome bot commented Dec 12, 2022

🎉 Congrats on merging your first pull request! 🥳 Looking forward to seeing more from you in the future! 💪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants