🌐 AI搜索 & 代理 主页
Skip to content

Conversation

@anntzer
Copy link
Contributor

@anntzer anntzer commented Mar 31, 2025

Another small preparatory PR for #29807 ({xe,lua}tex support).

Instead of directly exposing widths, heights, depths dicts, provide a get_metrics method to access a glyph's (tex) metrics. This change is in preparation for {xe,lua}tex support, which would use an alternative metrics-loading class (TtfMetrics) where it would be excessive to load the metrics of all glyphs at once.

PR summary

PR checklist

height: dict[int, int]
depth: dict[int, int]
def __init__(self, filename: str | os.PathLike) -> None: ...
def get_metrics(self, int) -> TexMetrics | None: ...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def get_metrics(self, int) -> TexMetrics | None: ...
def get_metrics(self, idx: int) -> TexMetrics | None: ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, fixed

@anntzer anntzer force-pushed the texmetrics branch 2 times, most recently from b09d728 to 3f2c7dd Compare March 31, 2025 11:13
@tacaswell tacaswell added this to the v3.11.0 milestone Mar 31, 2025
Copy link
Member

@dstansby dstansby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one suggestion to make the dataclass keyword argument only

@anntzer
Copy link
Contributor Author

anntzer commented Mar 31, 2025

sure, done

Instead of directly exposing widths, heights, depths dicts, provide a
get_metrics method to access a glyph's (tex) metrics.  This change is in
preparation for {xe,lua}tex support, which would use an alternative
metrics-loading class (`TtfMetrics`) where it would be excessive to load
the metrics of all glyphs at once.
@anntzer
Copy link
Contributor Author

anntzer commented Mar 31, 2025

I'm a bit at loss as to how to placate mypy here, so if someone who actually uses typing could have a look, this would be very helpful :)

@tacaswell
Copy link
Member

Pushed a commit that fixes it locally.

Look like this is a limitation in stubtests that does not correctly handle the kw_only=True functionality as __match_args__ is how dataclasses do some of their arg parsing in __init__.

@dstansby
Copy link
Member

Looks like it's related to (but not the same as) the fixed python/mypy#15749. Might be worth a minimal reproducer and a issue report upstream in mypy?

@dstansby
Copy link
Member

I've got a minimal reproducer, will post an issue upstream

@anntzer
Copy link
Contributor Author

anntzer commented Mar 31, 2025

Thanks :)

@tacaswell tacaswell merged commit 002661c into matplotlib:main Apr 1, 2025
41 checks passed
@tacaswell
Copy link
Member

thank you @dstansby !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants