Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

matplotlib use non-interactive agg backend #349

Merged
merged 5 commits into from
Nov 18, 2024

Conversation

DanielYang59
Copy link
Contributor

Fix a previously added parameter as the original comment is a bit misleading, agg is the recommended non-interactive backend:

The last, Agg, is a non-interactive backend that can only write to files. It is used on Linux, if Matplotlib cannot connect to either an X display or a Wayland display.

@DanielYang59 DanielYang59 marked this pull request as draft November 13, 2024 04:59
@DanielYang59
Copy link
Contributor Author

No this is not right, as running tests locally would behave differently than through GH action, would fix this later

@DanielYang59
Copy link
Contributor Author

DanielYang59 commented Nov 13, 2024

I realized the original method might be the best I could do so far. If we were to export the env var from the test workflow config file python-package.yml, it would only be effective running on actions (not locally).

However I believe having that MPLBACKEND set is still beneficial:

pytest doesn't seem to natively support setting environment variable from pyproject.toml and there is a plugin pytest-env just for this purpose (though with the price of added an additional tests dependency):

[tool.pytest_env]
HOME = "~/tmp"
RUN_ENV = 1

@@ -31,7 +31,6 @@ requires-python = ">=3.10,<3.13"
dependencies = [
"pymatgen>=2024.10.22",
"numpy<3.0.0",
"typing",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess we don't need to install typing now, it's built in from 3.5:

Added in version 3.5.

@DanielYang59 DanielYang59 marked this pull request as ready for review November 13, 2024 11:27
@naik-aakash
Copy link
Collaborator

Hi @DanielYang59 , before we merge, can you please provide a bit more context on why this change is needed and what are the issues when we don't have this?

I am not that familiar with lot of what matplotlib has to offer. So just want to understand bit better 😄

And thanks in anycase for this PR

@DanielYang59
Copy link
Contributor Author

DanielYang59 commented Nov 18, 2024

can you please provide a bit more context on why this change is needed and what are the issues when we don't have this?

Absolutely, I owe you an explanation here:

@naik-aakash naik-aakash merged commit 6143273 into JaGeo:main Nov 18, 2024
24 checks passed
@naik-aakash
Copy link
Collaborator

Thanks @DanielYang59

@DanielYang59 DanielYang59 deleted the mpl-backend branch November 18, 2024 07:21
@DanielYang59
Copy link
Contributor Author

No problem thanks for reviewing

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants