MRG: Revamp HTML reprs of `Raw`, `Epochs`, `Evoked`, and `Info`
For example, the Evoked repr contained the number of channels, but failed to subtract or report any bads.
Also I believe reporting the number of sampling points is quite pointless , so I removed that as well.
The table layout wasn't up-to-date either.
main (striped layout only added by VS Code, actually not present in the tables we produce!)
This PR:
For comparison, Epochs:
MWE:
# %%
import mne
sample_dir = mne.datasets.sample.data_path()
sample_fname = sample_dir / "MEG" / "sample" / "sample_audvis_raw.fif"
raw = mne.io.read_raw_fif(sample_fname)
raw.crop(tmax=60)
events = mne.find_events(raw, stim_channel="STI 014")
raw.pick("eeg")
epochs = mne.Epochs(raw, events=events, preload=True)
epochs
# %%
evoked = epochs.average()
evoked
This one also closes #12553
you're aligning to the minimum info by dropping number of channels and time points. I find it useful to know the duration and how many channels are available.
@agramfort Are you suggesting to add number of samples and channels to the Epochs repr instead, then? I could do that, no problem
yes I would have done that if you push for consistency.
Message ID: @.***>
Ok i'll take a look!
Ok, so I've now touched the Raw, Epochs, and Evoked HTML reprs to make things more consistent. Here's a comparison of what we have in main and what's in this PR branch now:
I'm generally quite happy with the improved consistency. Thanks! Nitpicks:
- I find "digitized head and sensor positions" doesn't quite make sense (the headshape points aren't "head positions" in the sense that we normally use that term). I'd revert to "digitized points"
- the "data kind" field is only in evoked; is that meant to communicate "avg" vs "stdev"? Does it make sense to collapse that into one field with
nave(something like "average of NN epochs" or "standard deviation of NN epochs")?
I'm generally quite happy with the improved consistency. Thanks! Nitpicks:
- I find "digitized head and sensor positions" doesn't quite make sense (the headshape points aren't "head positions" in the sense that we normally use that term). I'd revert to "digitized points"
I was worried this could be confused with "sampling points" on the time axis … I had first thought of labeling it "digitized had shape and sensor positions", but it's too long. I'm okay with changing it back to "digitized points", but it's not my favorite – maybe if we brainstorm a little, we can come up with a better idea?
- the "data kind" field is only in evoked; is that meant to communicate "avg" vs "stdev"?
Yes, it was there before and I'm actually not sure if it's really needed?
Does it make sense to collapse that into one field with
nave(something like "average of NN epochs" or "standard deviation of NN epochs")?
Good idea, I can do that!
@drammock Another thing we may need to address – these tables are quite long now, and I haven't added collapsible sections like we have for Raw (which essentially just prints out Info). Is that a problem for the tutorial rendering?
+1 for digitized points, unless someone comes up with a clearer alternative. Thanks for the upgrade, it looks great!
@mscheltienne I named the row now "Head shape and sensor digitization", and the value is then "xxx points" or "Not available"
WDYT?
@drammock Another thing we may need to address – these tables are quite long now, and I haven't added collapsible sections like we have for
Raw(which essentially just prints outInfo). Is that a problem for the tutorial rendering?
Ideally they'd have collapsible sections, yeah. Can you add them in this PR? If not please create a follow-up issue so someone can do it later.
Ideally they'd have collapsible sections, yeah. Can you add them in this PR?
This then also raises the question of how much Info (pun intended) to include here
For Raw, the HTML repr currently is literally Raw.info with recording filename and duration added.
For Epochs and Evoked, it's a mixture of some instance attributes plus some select parts extracted from inst.info.
If we add collapsibles anyway, we could add the entire Info (since it's easy to hide additional information that way), but I'm sure @cbrnr would hate it :)
Re collapsibles, I like the idea per se, but the current implementation for Info is visually unpleasant, since each section gets its own table and hence the columns don't align. I wonder if we can find a better solution.
Re collapsibles, I like the idea per se, but the current implementation for Info is visually unpleasant, since each section gets its own table and hence the columns don't align. I wonder if we can find a better solution.
I will look into this and propose a solution
I don't like the idea to misuse standard reprs of Python objects. You are only modifying HTML reprs, which I think is fine (and I don't use them anyway, so I really have no opinion on what should and should not be part of these tables).
Got you! @cbrnr
Looks like there are plenty of folks chiming in here with ideas so as long as you all converge I don't think I need to look, just wanted to say thanks for looking into this!
@larsoner If you have any thoughts on potentially a better label for "Digitized points", I'd love to hear it!
Maybe it's just me but in my ears it sounds kind of ambiguous / unspecific or even confusing when this information is presented in a summary table where we also display the number of time-series data sampling points
Any input welcome
Re collapsibles, I like the idea per se, but the current implementation for Info is visually unpleasant, since each section gets its own table and hence the columns don't align. I wonder if we can find a better solution.
I will look into this and propose a solution
I've found a solution, but it may take a day or two to implement. Will report back once I've had the time to do that
This is ready for review
I think a good way to explore these changes is by browsing through the Report tutorial, because the different examples there demonstrate the HTML repr for Raw, Epochs, and Evoked:
https://output.circle-artifacts.com/output/job/ce2dba84-e894-4f81-b168-a0b0af9807be/artifacts/0/html/auto_tutorials/intro/70_report.html#adding-raw-data
Note that for the documentation build, the repr sections are collapsed; during interactive use or when users create reports, all sections are expanded.
I consolidated repr sections across Info, Raw, Epochs, and Evoked.
Many helper functions were converted to Jinja2 filters, which allows us to do less "preparatory work" inside the _repr_html_() methods (e.g. for generating human-readable strings of datetimes); instead, this can now all be taken care of inside the Jinja2 templates, with only a few exceptions.
When displaying the recording time, I replaced the "GMT" timezone with "UTC", which I believe is more correct.
This PR contains one breaking API change: Info.ch_names used to raise an exception if Info["ch_names"] did not exist; now, it returns an empty list. This made certain operations for me easier. Since this should be a very rare edge case, I don't believe a deprecation cycle is necessary.
cc @agramfort
honestly having to click on each section to see something is to me an overkill. It forces me to use my mouse and while before shift+enter is enough to see what I want. Could we find a middle ground here? eg you see the important info by default and you can click to have the full details? or expand certain sections by default?
nit: the + sign is not to me the most common. It's more classic to have > and that become / when it's expanded.
Message ID: @.***>
@agramfort There might be a misunderstanding here, the clicking is only required in the documentation as is already the case with what we have in main
Edit: But you're right, I suppose even within the documentation the respective reprs should be expanded in the Report tutorial 🤔 But again, this problem is already present in main / stable
If instead you're using your Python Interactive window or creating a Report, everything is expanded by default, so no clicking is necessary!
Try this:
# %%
import mne
sample_dir = mne.datasets.sample.data_path()
sample_fname = sample_dir / "MEG" / "sample" / "sample_audvis_raw.fif"
raw = mne.io.read_raw_fif(sample_fname)
raw.crop(tmax=60).load_data()
events = mne.find_events(raw, stim_channel="STI 014")
raw.pick("eeg")
raw.set_eeg_reference(projection=True) # to add a proj
raw
# %%
epochs = mne.Epochs(raw, events=events, preload=True)
epochs
# %%
evoked = epochs.average()
evoked
# %%
raw.info
the + sign is not to me the most common. It's more classic to have > and that become / when it's expanded.
Yep, I will try to find a replacement
@agramfort
the + sign is not to me the most common. It's more classic to have > and that become / when it's expanded.
Yep, I will try to find a replacement
This has been resolved, see e.g.: https://output.circle-artifacts.com/output/job/9a976a2f-b617-4f47-9f43-8c4c1d998eae/artifacts/0/html/auto_tutorials/intro/70_report.html#adding-raw-data
Latest rendered Report tutorial: https://output.circle-artifacts.com/output/job/6a2f0a7d-04bb-4581-8c01-0b2ef7bdbbbe/artifacts/0/html/auto_tutorials/intro/70_report.html#adding-raw-data
CI errors seem unrelated
Dang, the carets (collapse / uncollapse buttons) don't show up on my iPad. Need to investigate.
I think this is ready for review now. I addressed @agramfort's concerns regarding the collapse / uncollapse buttons (they are carets now instead of plus / minus symbols). It seems to look ok (not perfect) in Reports on both desktop and my iPad, and in the VS Code Python Interactive console.
The segments of the repr are all expanded by default, and only collapsed in the built documentation (as is the case with our stable docs).
Rendered Report tutorial: https://output.circle-artifacts.com/output/job/bb7905a0-af6b-4d52-a43e-b196baa58e63/artifacts/0/html/auto_tutorials/intro/70_report.html#adding-raw-data
We still need to make a decision regarding the naming of the digitization points field.
cc @mscheltienne
Thanks for your feedback, @mscheltienne!
Looks great, the length of
Head shape and sensor digitizationis not bothering me.
I misread this as "is bothering me" and came up with a shorter proposal: "Head & Sensor Digitization" :) Thoughts?
It's actually a bit better :laughing:
@drammock I agree with all of these, including the naming of the "Data" section and the inconsistencies re the good / bad channels. However this is already present in main so I didn't really dare to touch it. But if we can come up with better ideas, I can certainly include them in this PR!
(Re Data: I actually spent a while thinking about it and exploring different names, but couldn't come up with anything better… happy to keep on brainstorming though!)
@drammock FYI when I started with this work, I named the Data section Recording, which seemed nice until I realized for an Info object, it seemed kinda odd, because it doesn't have any "recorded" data attached.
That said, one could argue that a Data section is just as inappropriate there.
So what do you say, shall we rename Data -> Recording? I can push a commit so you can take a look at the rendered reprs to get a better idea of what it "feels" like