Skip to content

Conversation

@eort
Copy link
Contributor

@eort eort commented May 7, 2021

Deals with this comment in #1234.

What does this implement/fix?

This PR adds the exclude keyword to raw.plot_psd() and epochs.plot_psd(). I essentially copied the exclude functionality from evoked.plot() to _psd_figure.py and fixed up the dependencies. Also two tests were added.

Additional information

pytest viz/tests/test_figure.py failed on my system, but it also did so on the main branch, so I don't think it is related to the changes.

@agramfort
Copy link
Member

thx @eort

can you see why CIs fail?

@eort
Copy link
Contributor Author

eort commented May 7, 2021

thx @eort

can you see why CIs fail?

Yeah, I guess so (see last commit)

@rob-luke
Copy link
Member

rob-luke commented May 8, 2021

I am getting that same Linux 3.8 CI fine cal error in #9375.

@eort
Copy link
Contributor Author

eort commented May 11, 2021

Looks good to me!

As I simply copied the code from the erp.plot(), should I port the suggested change here, to that function as well?

eort and others added 3 commits May 11, 2021 18:04
Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
@hoechenberger
Copy link
Member

What's the status here? This should be almost good to go after a rebase / merging main, no?

@eort
Copy link
Contributor Author

eort commented Oct 22, 2021

Not sure. I think the last commit was "ready", but I guess it was forgotten.

@hoechenberger hoechenberger self-assigned this Oct 22, 2021
@hoechenberger hoechenberger added this to the 0.24 milestone Oct 22, 2021
@hoechenberger
Copy link
Member

Sorry about that, @eort! Let's revive this.

@hoechenberger
Copy link
Member

It appears this has already been resolved by @cbrnr in #9519

Thank you for bringing up this issue and providing a first implementation, @eort, even if it didn't eventually make it into the main branch! Please keep your contributions coming 💪

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.

5 participants