Skip to content

Allow brave users to run PyGMT with old GMT versions? #1991

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

Closed
seisman opened this issue Jul 5, 2022 · 6 comments · Fixed by #2079
Closed

Allow brave users to run PyGMT with old GMT versions? #1991

seisman opened this issue Jul 5, 2022 · 6 comments · Fixed by #2079
Labels
enhancement Improving an existing feature
Milestone

Comments

@seisman
Copy link
Member

seisman commented Jul 5, 2022

Currently, the minimum required GMT version is 6.3.0 and it's likely that we will bump the minimum required GMT to 6.4.0 in the next few days (#1989).

After bumping to GMT>=6.4.0, users who use old GMT versions (e.g., GMT 6.3.0) won't be able to use future PyGMT versions unless they upgrade their GMT. Since most PyGMT functions still work with GMT 6.3.0, perhaps we should still allow users to run PyGMT with old GMT versions?

Currently, we raise an error and destroy the session:

pygmt/pygmt/clib/session.py

Lines 191 to 196 in edee1c4

if Version(version) < Version(self.required_version):
self.destroy()
raise GMTVersionError(
f"Using an incompatible GMT version {version}. "
f"Must be equal or newer than {self.required_version}."
)

Instead, we can raise a warning like

PyGMT requires GMT 6.4.0 but you're using GMT x.x.x. Some functions may not work as expected.
It's recommended to upgrade your GMT to >=6.4.0. Otherwise, use it at your own risk.

@seisman
Copy link
Member Author

seisman commented Jul 11, 2022

Ping @GenericMappingTools/pygmt-maintainers for thoughts on this.

@maxrjones
Copy link
Member

I agree with raising a warning rather than an error for most non-supported versions. However, we should raise an error if the version is truly not compatible (e.g., GMT 5).

@seisman seisman added enhancement Improving an existing feature and removed question Further information is requested labels Jul 11, 2022
@seisman seisman added this to the 0.8.0 milestone Jul 11, 2022
@weiji14
Copy link
Member

weiji14 commented Jul 11, 2022

Yes, I think the GMT 6.x series has gotten stable and reliable enough since 6.3.0, so we could allow for the next PyGMT version to run 6.3.0/6.4.0+ (though we did say we'll drop 6.3.0 in https://forum.generic-mapping-tools.org/t/pygmt-v0-7-0-released/3085, but hopefully nobody notices).

Just to clarify, this means that our CI will be using 6.4.0+, and there won't be any support or testing on GMT 6.3.0 correct?

@seisman
Copy link
Member Author

seisman commented Jul 15, 2022

this means that our CI will be using 6.4.0+,

Yes.

and there won't be any support or testing on GMT 6.3.0 correct?

I think we still need to test old GMT versions like GMT 6.3.0. We definitely can't run the full tests, because baselines images may change a lot among different GMT versions due to tiny upstream changes in gridlines or frames. Instead, we should at least test the core clib functions test_clib_*.py to make sure that loading the GMT library and passing data (vectors, matrix, strings) to GMT API still work as expected for old versions.

@seisman
Copy link
Member Author

seisman commented Jul 16, 2022

I think we still need to test old GMT versions like GMT 6.3.0. We definitely can't run the full tests, because baselines images may change a lot among different GMT versions due to tiny upstream changes in gridlines or frames. Instead, we should at least test the core clib functions test_clib_*.py to make sure that loading the GMT library and passing data (vectors, matrix, strings) to GMT API still work as expected for old versions.

PYGMT_USE_EXTERNAL_DISPLAY="false" pytest -m "not mpl_image_compare" pygmt

Using pytest -m "not mpl_image_compare" can skip all tests that have the @pytest.mark.mpl_image_compare decorator. Perhaps this is a better option to test old GMT versions.

@seisman
Copy link
Member Author

seisman commented Aug 23, 2022

Another simple way is running pytest without the --mpl option. In this way, all images are still generated (so we know they don't crash), but pytest doesn't do the image comparisons.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improving an existing feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants