mplfinance icon indicating copy to clipboard operation
mplfinance copied to clipboard

Using with `matplotlib.testing.decorators.image_comparison`

Open GarrisonD opened this issue 1 year ago • 6 comments

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 avatar Feb 25 '24 11:02 GarrisonD

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?

GarrisonD avatar Feb 25 '24 11:02 GarrisonD

@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.

DanielGoldfarb avatar Feb 25 '24 12:02 DanielGoldfarb

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

GarrisonD avatar Feb 25 '24 14:02 GarrisonD

@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

DanielGoldfarb avatar Feb 25 '24 17:02 DanielGoldfarb

@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!

GarrisonD avatar Feb 25 '24 20:02 GarrisonD

@GarrisonD sounds good. will take a look.

DanielGoldfarb avatar Feb 25 '24 20:02 DanielGoldfarb