mne-tools / mne-python

MNE: Magnetoencephalography (MEG) and Electroencephalography (EEG) in Python
https://mne.tools
BSD 3-Clause "New" or "Revised" License
2.7k stars 1.31k forks source link

Checkmarks in `mne.sys_info()` look out of place on Windows #12790

Closed cbrnr closed 1 month ago

cbrnr commented 2 months ago

Here's what I mean:

windows_sysinfo_symbols

Tested on Windows 11 in the default Terminal app.

larsoner commented 2 months ago

Extra confusing because the mne checkbox is different. I thought we used the same one for all packages that were installed.

Can we chalk this up to the Windows terminal being buggy? :smile:

Not sure if there would be a way to detect that we're in a "bad" Windows terminal instead of a "good" one (or even know which is which). Okay with me just to go with simpler unicode symbols assuming there are some that render properly in Windows cmd, bash within msys2, and Powershell (I assume those would be our primary targets?).

cbrnr commented 2 months ago

I don't think this depends on the shell, this should be related to the terminal app only. In the example, I've used the default Terminal app with the default font. IMO we should try to make it look decent for this use case (we cannot guarantee that for all combinations of terminals and fonts of course). So I guess looking for alternative symbols would be a good solution.

drammock commented 2 months ago

the MNE symbol is not a check, it's an X, so it's supposed to look different.

I suspect what's going on here is the windows terminal is using a fallback font to render some of the glyphs. I think the best we can do is look around and see if there's an older/simpler set of unicode "check / X / empty box" codepoints (ideally from the same codepage, but even that's not a guarantee that they'll all be present in a given font!) so that all the glyphs we use will have similar style/feel.

At a quick glance: https://en.wikipedia.org/wiki/Checkbox#Unicode there are 3 sequential code points that are "empty ballot box, X ballot box, and checked ballot box". And that is what we're already using. So I'm not optimistic that we'll be able to do any better, but feel free to look.

drammock commented 2 months ago

I suspect what's going on here is the windows terminal is using a fallback font to render some of the glyphs.

If you're able, you could test this by changing the font(s) used by the terminal, to a font that you know includes (similar-looking) glyphs for all 3 codepoints. This site lists "fonts that support this codepoint": https://www.fileformat.info/info/unicode/char/2612/fontsupport.htm

cbrnr commented 2 months ago

My point is that we should try to use symbols that look good with the default font (Cascadia Mono). In fact, 🗹 (ballot box with bold check) looks good! The empty box and the ballot box with x already look good! So maybe it's as simple as replacing that symbol (needs to be tested on other platforms though)?

drammock commented 2 months ago

My point is that we should try to use symbols that look good with the default font

agreed. I was trying to suggest (but I guess not clearly) that (1) changing the terminal font is a way to determine if a missing-glyph-in-primary-font + mismatched-glyph-from-fallback-font is really the source of the problem, and (2) fileformat.info is a resource for finding other fonts to try where you can be certain that all three glyphs are mapped.

I'm surprised that the bold check looks consistent while the regular check doesn't, since the bold check is from a much newer code block (in fact an entirely different code plane!) so I would have expected most fonts to be missing that glyph 🤷🏻

So maybe it's as simple as replacing that symbol (needs to be tested on other platforms though)

yep that's what I'd suggest pursuing

hoechenberger commented 2 months ago

image

fyi

safari, latest iOS

scott-huberty commented 2 months ago

safari, latest iOS

Same here (checkbox doesn't render; using Safari on macOS)

cbrnr commented 2 months ago

Given that ballot box with bold check fits perfectly with ballot box and ballot box with x, I'm inclined to think that it might be a bug in Cascadia Code; it looks like they might have mixed up "ballot box with check" and "ballot box with bold check".

The other alternative would be, dare I say it, get rid of all unicode symbols altogether.

cbrnr commented 1 month ago

And yes, @drammock is right that these symbols are not part of the font according to fileformat.info. Given that Cascadia is the default font in Windows Terminal and also Visual Studio Code (on Windows), I suggest that we remove these Unicode symbols and replace them with simple dashes.

Old:

Platform             macOS-14.6.1-arm64-arm-64bit
Python               3.12.5 (main, Aug  8 2024, 10:31:35) [Clang 15.0.0 (clang-1500.3.9.4)]
Executable           /Users/clemens/.pyenv/versions/mne/bin/python
CPU                  arm (12 cores)
Memory               Unavailable (requires "psutil" package)
Core
├☒ mne               1.8.0
├☑ numpy             2.1.1 (unknown linalg bindings (threadpoolctl module not found: No module named 'threadpoolctl'))
├☑ scipy             1.14.1
└☑ matplotlib        3.9.2 (backend=macosx)

Numerical (optional)
├☑ pandas            2.2.2
└☐ unavailable       sklearn, numba, nibabel, nilearn, dipy, openmeeg, cupy, h5io, h5py

Visualization (optional)
└☐ unavailable       pyvista, pyvistaqt, vtk, qtpy, ipympl, pyqtgraph, mne-qt-browser, ipywidgets, trame_client, trame_server, trame_vtk, trame_vuetify

Ecosystem (optional)
└☐ unavailable       mne-bids, mne-nirs, mne-features, mne-connectivity, mne-icalabel, mne-bids-pipeline, neo, eeglabio, edfio, mffpy, pybv

New:

Platform             macOS-14.6.1-arm64-arm-64bit
Python               3.12.5 (main, Aug  8 2024, 10:31:35) [Clang 15.0.0 (clang-1500.3.9.4)]
Executable           /Users/clemens/.pyenv/versions/mne/bin/python
CPU                  arm (12 cores)
Memory               Unavailable (requires "psutil" package)

Core
- mne               1.8.0
- numpy             2.1.1 (unknown linalg bindings (threadpoolctl module not found: No module named 'threadpoolctl'))
- scipy             1.14.1
- matplotlib        3.9.2 (backend=macosx)

Numerical (optional)
- pandas            2.2.2
- unavailable       sklearn, numba, nibabel, nilearn, dipy, openmeeg, cupy, h5io, h5py

Visualization (optional)
- unavailable       pyvista, pyvistaqt, vtk, qtpy, ipympl, pyqtgraph, mne-qt-browser, ipywidgets, trame_client, trame_server, trame_vtk, trame_vuetify

Ecosystem (optional)
- unavailable       mne-bids, mne-nirs, mne-features, mne-connectivity, mne-icalabel, mne-bids-pipeline, neo, eeglabio, edfio, mffpy, pybv

I've added a newline before the "Core" section to improve readability. WDYT?

hoechenberger commented 1 month ago

I'd like to keep the checkboxes, why can we not switch to a different unicode symbol? ✅ is what I commonly use, is this not rendering well in the Windows terminal?

cbrnr commented 1 month ago

I can only see a question mark when I copy and paste this symbol into the Terminal window. What's the Unicode code point? And which symbol would you use for an empty box?

hoechenberger commented 1 month ago

Then why don't we use Unicode on terminals that do and no unicode on terminals that don't support it? I thought we did this check somewhere in the code anyway, but perhaps it was removed at one point

cbrnr commented 1 month ago

Then why don't we use Unicode on terminals that do and no unicode on terminals that don't support it? I thought we did this check somewhere in the code anyway, but perhaps it was removed at one point

I don't know if we have this check, but would this work? Windows Terminal does support Unicode, it's just that the font doesn't have the symbols, and the replacements look really ugly.

drammock commented 1 month ago

I don't know if we have this check

indeed we do:

https://github.com/mne-tools/mne-python/blob/f3a3ca4430e1d4b9c539e7949c946f4f83bdb43f/mne/utils/config.py#L741

Windows Terminal does support Unicode, it's just that the font doesn't have the symbols, and the replacements look really ugly.

exactly right, the check is for the terminal program itself, not the font in use.

cbrnr commented 1 month ago

OK, so this means you're OK with getting rid of the symbols? I know you never liked them, whereas I usually prefer having Unicode symbols, but not if we don't find a solution that works on all platforms with a low maintainance effort.

drammock commented 1 month ago

OK, so this means you're OK with getting rid of the symbols? I know you never liked them, whereas I usually prefer having Unicode symbols, but not if we don't find a solution that works on all platforms with a low maintainance effort.

I don't feel strongly that we need to keep them, though this is one of the few places where I think they do offer an easier to read / more informative alternative to their ASCII equivalent (i.e., they're not just eye candy). So if you can find a way to keep them, I support that.

Perhaps ideal would be looking at the supported character sets for Cascadia Mono, finding glyphs that it does include that are good-enough equivalents to the ballot-box-{empty|checked|X-mark}, then either using those on all platforms (assuming they look ok) or triaging to use those only when on windows (or perhaps even only when on windows default terminal app). You could also consider triaging to just not use unicode when on windows default terminal app I guess.

However, I would caution that this effort is a moving target: we have no control over what font MSFT sets as the default, it may change, and likewise the default for other OSes / terminal apps may change. So it's really down to how much work you want to put into this. Honestly if it were me, I'd consider pinging @aaronbell to see if you can get the ballot box glyphs added to Cascadia, that seems like the closest we can get to a future-proof solution.

cbrnr commented 1 month ago

I would try to avoid triaging and rather use the same symbols on all platforms. Having said that, the only viable symbols in Cascadia Mono seem to be these square boxes:

Screenshot 2024-09-09 at 16 37 28

I could try and see if these are also supported on macOS and Linux, but as you correctly mentioned, this is always going to be a moving target (since there is no default Terminal font on Linux, this depends on the distribution and the desktop environments).

Personally, I'd try to minimize our efforts, it doesn't seem to be worth it. The output is just as easy to read without symbols IMO.

drammock commented 1 month ago

the only viable symbols in Cascadia Mono seem to be these square boxes

yep, at a quick glance I had also noticed those. There's also a plain checkmark (next to the card-suit symbols, not shown in your screenshot).

it doesn't seem to be worth it. The output is just as easy to read without symbols IMO.

Perhaps then the simplest way forward is to change the default to sys_info(..., unicode=False)?

aaronbell commented 1 month ago

If I may chime in here, there is no single, well-respected character set for all coding fonts to follow, so as long as you’re reliant on the font to display a given character you’ll always run the risk of having fallback and inconsistent rendering. Feel free to file a bug on Cascadia and I can probably get the glyphs added in the next update, though.

That said, to avoid a situation where you’re seeing fallback from another font, you might want to consider maintaining a list of Unicode symbols and have an acceptable fallback glyph option(?) for them. That way if the font does support the particular glyph, you’ll get it, and if not you get the acceptable alternative.

drammock commented 1 month ago

Thanks for chiming in @aaronbell!

as long as you’re reliant on the font to display a given character you’ll always run the risk of having fallback and inconsistent rendering

yep, exactly what I was trying to say above (thanks for saying it more clearly!)

if the font does support the particular glyph, you’ll get it, and if not you get the acceptable alternative

That sounds... hard. I think it would involve:

So I think I'm still going to advocate that @cbrnr take the approach of (1) changing our default to ASCII-only output for now, and (2) opening an issue to hopefully get the 3 glyphs we need added to Cascadia.

larsoner commented 1 month ago

Rather than unicode=False I suggest unicode="auto" which triages based on OS, which I think (?) would be "True on Linux and macOS and False on Windows". That seems to be like a good balance to get the nicest behavior possible most often without too much magic.

cbrnr commented 1 month ago

Sounds good, I'll implement @larsoner's suggestion then, and we can internally change the behavior of "auto" on Windows whenever the new characters are available in the font.