mne-qt-browser icon indicating copy to clipboard operation
mne-qt-browser copied to clipboard

New channel type scaling setting diffes from ViewBox scaling display

Open mscheltienne opened this issue 1 year ago • 8 comments

Describe the problem

The added channel type scaling in #263 is half of the display channel type. It would be nice to harmonize and choose which one to display.

Screenshot from 2024-07-02 15-55-18

@larsoner @nmarkowitz Any strong opinion here?

mscheltienne avatar Jul 02 '24 13:07 mscheltienne

@mscheltienne I'm not sure what you mean by this

nmarkowitz avatar Jul 02 '24 14:07 nmarkowitz

grad (ft/cm) in the settings shows 400; mark on the viewbox shows 800 fT/cm.

mscheltienne avatar Jul 02 '24 14:07 mscheltienne

ah I see. but that is also controlled by the independent mne.scale_factor property. Currently the formula for displaying that text is (self is a ScaleBarText instance and ch_type is any type of channel)

scaler = 1 if self.mne.butterfly else 2
inv_norm = (
    scaler
    * self.mne.scalings[self.ch_type]
    * self.mne.unit_scalings[self.ch_type]
    / self.mne.scale_factor
)
self.setText(f"{_simplify_float(inv_norm)} " f"{self.mne.units[self.ch_type]}")

The scale factor is adjusted by the eyeglass icons at the top of the browser. We can remove those and the user will have to control the scaling of each channel type manually. Or add a checkbox for whether to use the scale factor or purely use manual adjustment

Screenshot 2024-07-02 at 11 54 29 AM

nmarkowitz avatar Jul 02 '24 15:07 nmarkowitz

One option would be to get rid of self.mne.scale_factor completely and when the "scale all up" or "scale all down" buttons are changed, change the self.mne.scalings instead.

But that's a bit of a separate issue from whether or not the magenta scale bar matches the chosen scale factor. I think it probably should. In other words, it's twice as large as it should be at the moment, so we can just make it half the size (it could extend upward from the baseline/zero, or extend halfway above and halfway below the baseline).

larsoner avatar Jul 02 '24 16:07 larsoner

Should this be fixed in https://github.com/mne-tools/mne-qt-browser/pull/268 or a separate PR?

nmarkowitz avatar Jul 02 '24 18:07 nmarkowitz

Could go either way... I would probably keep them separate until we converge on how to harmonize. In the meantime we can know that there should be a factor of 2x difference between the two when testing interactions in #268

larsoner avatar Jul 02 '24 19:07 larsoner

I would do 2 separate PRs

mscheltienne avatar Jul 02 '24 19:07 mscheltienne

I fixed this in commit https://github.com/mne-tools/mne-qt-browser/pull/268/commits/ee2d1aba5188003323aac897ad327ec845b080ce . It was an issue with how I was calculating scale. I wasn't using complete formula. It's now synchronized.

I did find another error. When butterfly mode is toggled scale bar text isn't updated. I'll make a commit for that

nmarkowitz avatar Jul 03 '24 02:07 nmarkowitz