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

Using with matplotlib.testing.decorators.image_comparison #661

Open
GarrisonD opened this issue Feb 25, 2024 · 6 comments
Open

Using with matplotlib.testing.decorators.image_comparison #661

GarrisonD opened this issue Feb 25, 2024 · 6 comments
Labels
question Further information is requested

Comments

@GarrisonD
Copy link

The problem is that the baseline images generated locally (in a Dev Container) don't match the ones on CI (GitHub Actions running the latest Ubuntu LTS) and thus my tests fail. When I compared the images I figured out that different fonts are being used: CI's Ubuntu has more fonts pre-installed than a Dev Container so the text was rendered with Liberation Sans instead of DejaVu Sans.

To quick fix it:

  1. Install Liberation fonts locally
    sudo apt install fonts-liberation
  2. Drop matplotlib fonts cache (file location and name can be different)
    rm ~/.cache/matplotlib/fontlist-v330.json
  3. Re-generate baseline images

Also to find a solution that won't fail when some font is added/removed locally/remotely, I dug deeper and figured out that matpltolib has a solution for this well-known issue: matplotlib.testing.setup() that calls set_font_settings_for_testing() that sets font.family to DejaVu Sans.

But for mplfinance that makes no difference because the font.family gets overridden by _apply_mpfstyle call. For example, the default style sets it to sans-serif (comes from base_mpl_style='seaborn-darkgrid'), and then nobody knows which font maptlotlib is going to use for rendering.

Here I came up with two solutions:

pytest fixture

Pros:

  • uses only public API

Cons:

  • need to pass to every .plot(...)
@pytest.fixture
def mpf_style():
    return mpf.make_mpf_style(
        base_mpf_style="default", rc={"font.family": "DejaVu Sans"}
    )

@image_comparison(baseline_images=["test.png"])
def test(mpf_style):
  mpf.plot(..., style=mpf_style)

pytest before-all hook that mutates default style

Pros:

  • all the existing tests left untouched
  • never forget passing style=... in new ones

Cons:

  • gets broken on the underlying mplfinance implementation changes
def pytest_configure() -> None:
    mpf._styledata.default.style["rc"].append(("font.family", "DejaVu Sans"))
@GarrisonD GarrisonD added the question Further information is requested label Feb 25, 2024
@GarrisonD
Copy link
Author

I spent some hours investigating the issue and looking for solutions.
I think it's worth mentioning in the Testing section of README.md.

@DanielGoldfarb What do you think?

@DanielGoldfarb
Copy link
Collaborator

@GarrisonD Thanks for putting time into this. Just a quick glance at what you wrote above, sounds like a similar issue I had, a year or two ago, when I upgraded Ubuntu and it took me a while to realize that the upgrade had a different set of fonts.

I will try later this week to make time to look into this in more detail. In the meantime, can you please describe in some detail the steps you went through that initially led you to find this problem? Thanks.

@GarrisonD
Copy link
Author

@DanielGoldfarb Are you familiar with Docker? I want to create a reproducible setup and share it with you.

@DanielGoldfarb
Copy link
Collaborator

@GarrisonD
I know what docker is, and understand what it does, but I have never used it. It's been on my list of things to learn, so please go ahead and create a docker image for me and I will do my best to make time this week to use it. Thanks. --Daniel

GarrisonD added a commit to GarrisonD/mplfinance that referenced this issue Feb 25, 2024
@GarrisonD
Copy link
Author

@DanielGoldfarb Sorry, but no Docker this time... I found a better option 😄

Just open https://github.com/GarrisonD/mplfinance, create a new GitHub Codespace, and follow README.md

If I can help you with GitHub Codespaces, the code, testing, or whatever, just let me know!

@DanielGoldfarb
Copy link
Collaborator

@GarrisonD sounds good. will take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants