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

MNT: helper function for checking for presence of fonts in tests #27983

Closed
story645 opened this issue Mar 27, 2024 · 4 comments
Closed

MNT: helper function for checking for presence of fonts in tests #27983

story645 opened this issue Mar 27, 2024 · 4 comments

Comments

@story645
Copy link
Member

story645 commented Mar 27, 2024

As mentioned in #27121, some of the tests could be cleaned up if there was a function to check for the presence of fonts.

I guess we really just need a helper in font_manager that does basically the same thing as findfont, but returns None (or raises) when the font of the requested family is not found, without ever trying to return a font from another family or performing fallback?

Originally posted by @anntzer in #27128 (comment)

@tacaswell
Copy link
Member

I think we can close this with no action because findfont already has a keyword arguement that does this:

imPython 3.11.8 (main, Feb 12 2024, 14:50:05) [GCC 13.2.1 20230801]
Type 'copyright', 'credits' or 'license' for more information
IPython 8.22.2 -- An enhanced Interactive Python. Type '?' for help.

In [1]: import matplotlib.font_manager as fm

In [2]: fm.findfont('noro')
findfont: Font family ['noro'] not found. Falling back to DejaVu Sans.
Out[2]: '/usr/lib/python3.11/site-packages/matplotlib/mpl-data/fonts/ttf/DejaVuSans.ttf'

In [3]: fm.findfont('noro', fallback_to_default=False)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Cell In[3], line 1
----> 1 fm.findfont('noro', fallback_to_default=False)

File /usr/lib/python3.11/site-packages/matplotlib/font_manager.py:1296, in FontManager.findfont(self, prop, fontext, directory, fallback_to_default, rebuild_if_missing)
   1292 ret = self._findfont_cached(
   1293     prop, fontext, directory, fallback_to_default, rebuild_if_missing,
   1294     rc_params)
   1295 if isinstance(ret, _ExceptionProxy):
-> 1296     raise ret.klass(ret.message)
   1297 return ret

ValueError: Failed to find font noro:style=normal:variant=normal:weight=normal:stretch=normal:size=10.0, and fallback to the default font was disabled

In [4]:

@story645
Copy link
Member Author

story645 commented Apr 5, 2024

ValueError is not the same as a true false b/c it requires wrapping the code in a try/catch. Which is why

but returns None

@story645
Copy link
Member Author

story645 commented Apr 5, 2024

I think @timhoffm's solution of testing helper works #28028 (comment)

@story645 story645 changed the title MNT: helper function for checking for presence of fonts MNT: helper function for checking for presence of fonts in tests Apr 5, 2024
@tacaswell
Copy link
Member

I still think we should close this with no-action. If we need such a helper in the tests, then we should write it when we need it.

Having an unused helper around does not do any good and going through and adding code-churn to the tests just seems like unneeded work.


I do not think that returning a font or None is the best API here. It does let you do things like

if font := hypothetical_find_font(...):
    use(font)

so it makes sense for something like re.match where not finding something is "normal" as macth is "is there a match, and if so what is it?". On the other hand in our case if we ask for a font and can not find the font...we are out of luck and should raise as failing to find a font when you need one is truly exceptional!

If you want to run your own fallback you can write it like

for name in candidates:
    try:
        return fm.findfont(name, fallback_to_default=False)
    except ValueError:
        continue
else:
    raise ValueError(f"no fonts found of {candidates}")

Returning None also leaves open the risk of something like

font = hypothetical_find_font(...)
# for got to check!
...
# many LoC
...
function_that_uses(font) # got AttirubteError: None type has no attribute 
# ... in some other part of the code

so the source of the problem is well away from where the traceback will point you.

@story645 story645 closed this as completed Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants