Move plotting functions to separate modules

[willschlitzer]

As mentioned in #685, this is to split up base_plotting.py and move the plotting functions to separate scripts, following the method used by @weiji14 in #686.

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.

Notes

• You can write /format in the first line of a comment to lint the code automatically

[willschlitzer]

@GenericMappingTools/python I moved all of the plotting functions to their own modules. I'm running into a few style errors that I'm not sure how to address.

pygmt/src/contour.py:98:13: W0212: Access to a protected member _preprocess of a client class (protected-access)
I'm guessing this is because the functions use _preprocess but don't import it, but the tests successfully running makes me assume that it works because _preprocess is in the class that these modules are imported into.

pygmt/base_plotting.py:8:0: R0903: Too few public methods (0/2) (too-few-public-methods)
I'm assuming this is because the class doesn't really "do" all that much specifically, since the functions are all housed in their own modules. I'm assuming this is acceptable, otherwise functions would just be arbitrarily moved back to base_plotting.py.

pygmt/src/text.py:48:4: W0621: Redefining name 'text' from outer scope (line 40) (redefined-outer-name)
I'm guessing "text" is a common/special word, so having it in the name of both a function and module is causing difficulties. Not sure how it should be addressed.

[weiji14]

As mentioned in #808 (comment), could you delete base_plotting.py (can't believe I'm saying this!) and move this import chunk to figure.py? This will fix the pylint R0903: Too few public methods (0/2) (too-few-public-methods) recommendation. Feel free to copy any of the docstrings if needed.

[willschlitzer]

@weiji14 I just moved this import string to figure.py and removed imports for base_plotting.py, but when I ran make test I had 196 test failures. I'm not sure what the need for _preprocess is, but it seems to play some sort of role.

[weiji14]

Just double checking that you placed the imports under class Figure():? I.e. indented 4 spaces? The _preprocess class function activates the correct figure to plot in.

[willschlitzer]

...nope 🤦‍♂️

Removed base_plotting.py in 5ac2e07 and actually put the imports in Figure() and it worked (or at least passed all of the tests)!

[seisman]

As mentioned in #808 (comment), could you delete base_plotting.py (can't believe I'm saying this!) and move this import chunk to figure.py? This will fix the pylint R0903: Too few public methods (0/2) (too-few-public-methods) recommendation. Feel free to copy any of the docstrings if needed.

We should be careful with the changes. There are two different _preprocess() functions in Figure() and BasePlotting. If I understand it correctly, the _preprocess function if BasePlotting override the one in Figure().

[willschlitzer]

@seisman The tests are passing; is there an issue with deleting base_plotting.py?

[weiji14]

We should be careful with the changes. There are two different _preprocess() functions in Figure() and BasePlotting. If I understand it correctly, the _preprocess function if BasePlotting override the one in Figure().

I feel like the _preprocess in base_plotting.py was just a dummy placeholder, and the one in figure.py is the actual one used. So probably the other way around? Might need to read up on subclassing, I'm not terribly good at object orientated conventions.

Edit: Yep, the _preprocess in Figure() takes precedence over BasePlotting. See https://stackoverflow.com/questions/12764995/python-overriding-an-inherited-class-method

[seisman]

I thought BasePlotting() is a subclass of Figure(), now it makes sense to me.

[weiji14]

I think this is good to merge now (after updating the branch), what do you think @seisman?

Deleting base_plotting.py and moving every module to src will cause conflicts in several open PRs (e.g. velo at #525, triangulate at #731, the grdimage work at #750, etc), but we can sort those out later 🙃

[seisman]

Looks good to me, except a few tiny suggestions.