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

[WID-218] Add controls for channel coloring - TS widget with plotly #67

Merged
merged 5 commits into from
Apr 17, 2024

Conversation

peeplika
Copy link
Contributor

Added a dropdown menu to the TimeSeriesWidgetPlotly for selecting colormap for individual channels.

@romina1601
Copy link
Member

Hi @peeplika
The dropdown looks ok, but I have some notes on it:

  • If you select just a few channels, then change the color scheme from the dropdown, all the channels are displayed, but we should only see the selected channels
  • Same thing happens if you select a few channels, then change the signal scaling. See the screenshot from below

image

  • I don't think all the color schemes are suited for this type of visualization, because sometimes some channels are not really visible (e.g.: "RdGy" color scheme, where some channels are almost white). See the bottom channels in screenshot from below.
    image

  • I found at least one color scheme that doesn't seem to work (it's called "rainbow"). There is simply no change in the colors when I select it.

  • Changing the colors is a bit too slow. Would be great if you could make an improvement on that part as well.

@peeplika
Copy link
Contributor Author

Hello @romina1601,

Thank you for your feedback. I've made several improvements based on your notes:

1.Fixed the issue where all channels were displayed when changing the color scheme.
2.Removed unsuitable colormaps.
3.Enhanced the speed of color scheme changes.

I have also added an option for uni-color.
I kindly ask you to review the changes and let me know if any further adjustments are needed.

@romina1601
Copy link
Member

Hi @peeplika ,

  1. The first improvement that you made doesn't seem to work for me. The issue persists: I select a few channels, hit Submit selection, then change the Color map or Signal scaling and all the channels reappear on the plot.
  2. This looks good, now I can clearly see all the channels.
  3. It is indeed faster, which is great.

@peeplika
Copy link
Contributor Author

peeplika commented Apr 4, 2024

Hi @romina1601
Thanks for the review.
I have made a few changes - Only the selected channels are passed as arguments in update_colormap and update_scaling methods so that only those are displayed. I have renamed picks as ch_picked for clarity and consistency.
I think the issue should be resolved now . Can you please take a look again ?

@romina1601
Copy link
Member

Hi @peeplika ,
Looks good now and it moves fast enough. But there seems to be a problem with the colormap "gist_ncar_r". If I select it, it results in an error. Could you take a look at that?

@romina1601
Copy link
Member

Hi @peeplika , Looks good now and it moves fast enough. But there seems to be a problem with the colormap "gist_ncar_r". If I select it, it results in an error. Could you take a look at that?

This is a problem only in the TimeSeriesTVB.ipynb notebook. In the other notebook (the one using numpy arrays) it works fine.

@peeplika
Copy link
Contributor Author

Hi @romina1601
Many thanks for reviewing again.
I have made some adjustments to prevent the error

@liadomide liadomide merged commit fcc188d into the-virtual-brain:main Apr 17, 2024
0 of 3 checks passed
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.

3 participants