Refactor xfail tests to avoid storing baseline images

[seisman]

Description of proposed changes

This PR refactors the xfail/fail tests to let the tests pass and avoid storing baseline images.

The baseline images are generated by calling the same modules using plain text or grid files are input, and using single-character arguments.

Address #451.

Remaining issues:

• The two xfailing tests in test_config are not refactored. There is no easy way to generate references images for them.

TODOs for future PRs:

• Tests that are passing are not refactored. These tests still use the images in the pygmt/test/baselines directory as baselines.
• Images of those xfailing/failing tests are no longer needed, but this PR doesn't delete them from the repository.

Reminders

• Run make format and make check to make sure the code follows the style guide.
• Add tests for new features or tests that would have caught the bug that you're fixing.
• Add new public functions/methods/classes to doc/api/index.rst.
• Write detailed docstrings for all functions/methods.
• If adding new functionality, add an example to docstrings or tutorials.

[seisman]

The reference image is created by directly calling the low-level API function call_module.

[weiji14]

I like the concept in general (testing against the low level GMT API), but this can get complicated real fast for some modules that output to a file (see e.g. @liamtoney's example at #364 (comment) for grdgradient). Though to be honest, that's probably not what this PR is about.

Should we have a convenience function to wrap around this low-level API better? E.g. something like pygmt.gmt(module="basemap", arg_str="-R0/360/0/1000 -JP6i -Bafg") that removes the with` statement.

[seisman]

but this can get complicated real fast for some modules that output to a file (see e.g. @liamtoney's example at #364 (comment) for grdgradient).

Most tests are simple, testing only one aspect of functions. #364 (comment) example may be the most complicated one, but we won't use virtual files for the references images.

Should we have a convenience function to wrap around this low-level API better? E.g. something like pygmt.gmt(module="basemap", arg_str="-R0/360/0/1000 -JP6i -Bafg") that removes the with` statement.

I believe @leouieda is against it (#122 (comment)).

[weiji14]

Most tests are simple, testing only one aspect of functions. #364 (comment) example may be the most complicated one, but we won't use virtual files for the references images.

True, but as you mentioned, this way of testing isn't very Pythonic or new contributor friendly. We can definitely think of a better solution that doesn't involve clib here, just need a bit more time, and maybe have a look around some other projects like GMT.jl and matplotlib to see how they do it.

I believe @leouieda is against it (#122 (comment)).

Ah yes, I thought I saw this somewhere before, thanks for linking. Definitely hold on the pygmt.gmt/fig.rawcommand feature then.

[weiji14]

Just as a throwaway idea, what about testing that the long aliases and list-to-str conversion works? I.e. have fig_test use long aliases like region, and have fig_ref use the short alias like R.

Suggested change

	fig_test = Figure()
	fig_test.basemap(R="0/360/0/1000", J="P6i", B="afg")
	fig_ref = Figure()
	with Session() as lib:
	lib.call_module("basemap", "-R0/360/0/1000 -JP6i -Bafg")
	fig_test = Figure()
	fig_test.basemap(region=[0, 360, 0, 1000], projection="P6i", frame="afg")
	fig_ref = Figure()
	fig_ref.basemap(R="0/360/0/1000", J="P6i", B="afg")

[seisman]

Sounds a clever idea. I believe we are confident that arguments like R="0/360/0/1000", J="P6i", B="afg" will be converted to "-R0/360/0/1000 -JP6i -Bafg", thus, fig_ref.basemap(R="0/360/0/1000", J="P6i", B="afg") is the same as lib.call_module("basemap", "-R0/360/0/1000 -JP6i -Bafg").

[weiji14]

Cool, if you want to update this PR to fix the failing 'polar', 'logo' (as mentioned at #451 (comment)) tests, and any other ones that pop up (I saw a few more upstream GMT changes already...), I'll review it later.

[seisman]

OK. I'll work on this PR.

[weiji14]

A lot to review, but it's looking good! Just a couple of comments.

The two xfailing tests in test_config are not refactored. There is no easy way to generate references images for them.

Images of those xfailing/failing tests are no longer needed, but this PR doesn't delete them from the repository.

Yes, will need to remember to do these in the future as separate PRs.