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

Update ft_plot_layout #2344

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

NirOfir
Copy link
Contributor

@NirOfir NirOfir commented Nov 15, 2023

I tried to improve a bit the doc of ft_plot_layout, mainly to make clearer you can set different colors, symbols or sizes per marker (#1116). Additionally, I tried to make parameter expansion a bit more robust, so that it expands any of the three marker parameter (symbol, size and color). It would expand previously only if the number of colors specifically was more than 1.

Filled in missing lines and tried to make the doc more helpful generally.
Make parameter expansion a bit more robust, so that it expands any of the three marker parameter (symbol, size and color). It would expand previously only if the number of colors specifically was more than 1.
Copy link

You should test whether your modifications do not break anything.
See https://www.fieldtriptoolbox.org/development/testing/

When outside the DCCN, please consider testing: test_ft_prepare_layout, test_ft_plot_layout

@NirOfir
Copy link
Contributor Author

NirOfir commented Nov 15, 2023

I looked at the suggested tests. Both only call ft_plot_layout(layout), that is without additional parameters, so they won't be affected by the change.

@NirOfir
Copy link
Contributor Author

NirOfir commented Nov 16, 2023

I looked through the codebase and found 3functions that call ft_plot_layout with marker properties:
ft_multiplotER with viewmode = butterfly
ft_singleplotER with showlocations = true
topoplot_common with highlighted channels
All work for me with the modified code.

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.

None yet

1 participant