Enhancement in the gridline options

[PhilipeRLeal]

Title: Enhancement in the gridline options:

Rationale

This update in the gridline module (plus a formatter module) allows cartopy to set the hemisphere units according to the user requirements. For instance, West hemisphere symbol is "W" as default. Nevertheless, this default can be changed by the user to any other symbol, like the letter "O" (Oeste). This asset can expand the applicability of the cartopy package to other countries for Map creation (i.e.: latin countries).

All hemisphere units can be changed by the user: North-South and West-East

Also, decimal separation symbol ("dots") can now be changed to other symbols (i.e.: "commas").

Implications

None Applicable

Examples:

Code for creating the below figures:

import matplotlib.pyplot as plt
import cartopy.crs as ccrs

def test_gridliner_change_gridline_tick_decimal_separator():
    plt.figure()
    ax = plt.subplot(1, 1, 1, projection=ccrs.PlateCarree())
    ax.stock_img()
    ax.set_extent([-80, -40.0, 10.0, -30.0])
    gl = ax.gridlines(draw_labels=True)
    try:
        gl.change_gridline_tick_decimal_separator(
            gl_num_format='{0:.2f}',
            axis='both',
            decimal_separator=',')

        Result = True
    except BaseException as e:
        print(e)
        Result = False

    assert(Result)


def test_gridliner_hemisphere_ticklabels_formatting():
    plt.figure()
    ax = plt.subplot(1, 1, 1, projection=ccrs.PlateCarree())
    ax.stock_img()
    ax.set_extent([-80, -40.0, 10.0, -30.0])
    gl = ax.gridlines(draw_labels=True)
    try:
        gl.set_longitude_hemisphere_str(west_hemisphere_str='O',
                                        east_hemisphere_str='L')

        gl.set_latitude_hemisphere_str(north_hemisphere_str='N',
                                       south_hemisphere_str='S')

        Result = True
    except BaseException as e:
        print(e)
        Result = False

    assert(Result)

if '__main__' == __name__:
    test_gridliner_change_gridline_tick_decimal_separator()
    test_gridliner_hemisphere_ticklabels_formatting()

Maps with custom labels and decimal separators.

1. Map with commas as decimal separators:

2. Map in accordance to latin country standards: comma decimal separated, and west-east hemispheres with latin symbols (O and L).

[PhilipeRLeal]

Dear all, I don't understand the error messages returned by the testers. Could anyone translate them to me? When I run this Pull request in my computer, everything runs smoothly.

Sincerely,

[greglucas]

The tests that are failing are known failures. But, it also looks like you deleted some tests in your commits, please don't do that, the tests are there for a reason of expecting certain behavior.

Overall this approach seems quite verbose for what you're trying to do.

Does the matplotlib rcParam axes.formatter.use_locale work for the decimal/comma conversion?

#Locale settings
import locale
locale.setlocale(locale.LC_ALL, "fr_FR")
import matplotlib as mpl
mpl.rcParams['axes.formatter.use_locale'] = True

My other thought is that updating 4 separate values for each hemisphere seems like a lot. What if instead, we had a cardinal_directions keyword argument or something like that for only one parameter to update? cardinal_directions="NSEW" as the default and cardinal_directions="NSOL" for your case above?

[PhilipeRLeal]

Dear @greglucas , thank you for your reply.

Regarding your first question

"Does the matplotlib rcParam axes.formatter.use_locale work for the decimal/comma conversion?)"

The simple answer is no. I tryied everything I knew in order to change or modify the gridline ticklabels' decimal separator, yet nothing worked.

The only solution I found was to properly set it, as a new function, which is proposed in this Pull Request.

Regading the second question/suggestion:

It is well possible to update the 4 separate values for each hemisphere in a single shot.

The idea of cardinal directions is actually great. I believe that one could provide that in several ways:

1. as a full list: ['N', 'S', 'E', 'W']
2. as a string: "NSEW"
3. as a dictionary: {'left_label_symbol' : 'W', 'right_label_symbol' : 'E', 'top_label_symbol' : 'N', 'bottom_label_symbol' : 'S'}
4. or as properties for an independent class (i.e.: CardinalDirections)

I will make some adjustments for this new CardinalDirections approach, and commit a new version for this Pull Request (PR). I ask you to check this new commit (as soon as submitted), and check if this new changes are more in agreement with cartopy's programming policies.

Regarding the deleted test files that were generating errors:

I wasn't sure if I should keep them, specially since they are not working. As I noticed on their error messages (returned by the test set), they always cause unsuccessful tests. Therefore, It becomes nearly impossible to check if this Pull Request's commits are alright, or not.

In this last commit, I tried to retrieve all possible tests that were present in cartopy's master branch. That is why the test sets returned error (I think). Can someone help me confirm this statement?

Sincerely,

[PhilipeRLeal]

I have updated the package in accordance to the requests made by @greglucas.

In respect to third parties modules (which are not applied by this pull request), the test suit seems to be causing some failures. I can't be sure of it. Can someone confirm that?

Sincerely,

[stefraynaud]

I agree with @greglucas I think playing with the locale is the probably the best approach. It seems that it would requires only to use locale.string_format when formatting the degrees here: https://github.com/SciTools/cartopy/blob/32b203b526475c942806f4b8485414f72fb49f98/lib/cartopy/mpl/ticker.py#L161

As for the cardinal names, why not just adding an option to the current formatters?

[PhilipeRLeal]

Dear @stefraynaud,

thank you for your reply.

As it seems, the locale has some internal Issues (especially with Latin symbols) which are not easily solved during programming time. One special case is when one applies Xticks/Yticks Formatters and Locattors in the gridline.

I have tested several alternatives, prior to this Pull Request contribution, and the only solution I found so far was to implement this proposed approach. With this Pull Request, it is now easily convertible the cardinal symbols, and respective decimal separators (i.e.: comma or period). No impact on the main cartopy's gridline functionality has been observed.

[stefraynaud]

Thank you for your answer.

To better understand, I made very little modifications to the current implementation of the formatters: https://github.com/SciTools/cartopy/compare/master...stefraynaud:decimal_point?expand=1 With these modifications, the decimal point can be modified by the locale setting or the decimal_point new option. then, it is crudly used as replacement of the current dot when formatting the degrees. And a cardinal_labels option is added and takes a dict whose keys are east, north, west and south.

import matplotlib.pyplot as plt
import cartopy.crs as ccrs
import cartopy.mpl.ticker as cticker

# import locale
# import matplotlib as mpl
# locale.setlocale(locale.LC_ALL, "fr_FR")
# mpl.rcParams['axes.formatter.use_locale'] = True

ax = plt.subplot(1, 1, 1, projection=ccrs.PlateCarree())
ax.stock_img()
ax.set_extent([-80, -40.0, 10.0, -30.0])

gl = ax.gridlines(draw_labels=True, dms=False )
gl.xformatter = cticker.LongitudeFormatter(decimal_point=',', cardinal_labels={'west': 'O'}, number_format='0.2f')
gl.yformatter = cticker.LatitudeFormatter(decimal_point=',', number_format='0.2f')
plt.show()

This code produces the figure above. It is quite easy to expose these options to the gridlines method.

This is a very basic implementation that should fail in some cases, sorry for that, but does this meet some of your needs or is it off topic?

[PhilipeRLeal]

Dear @stefraynaud,

thank you for your contribution.

As it seems, your proposed change will do exactly what my Pull Request does.

As you have mentioned, there may be some potential failures within your code.

If they can be properly solved and effectively tested, I don't see why not use your proposal, instead of mine.

Therefore, if you (or any other contributor) wish to apply this Pull Request proposed enhancements into Cartopy's master Branch, feel free to opt between the two alternatives.

Sincerely,

[stefraynaud]

@PhilipeRLeal my little was just here better understand things and propose some way to not create now module and declare to classes. It even don't pass the test suite and has no specific test, so I won't make any PR because I think the goal of your PR is very important.

[PhilipeRLeal]

Dear @stefraynaud,

I finally understood your point. I thank you for your suggestions.

Nevertheless, for the meanwhile, I will keep this Pull Request open until further notice.

Sincerely,

[PhilipeRLeal]

Dear Developers,

As the CI (Continuous Integration) reported, there was an error in the test build. Nevertheless, this error was due to some unknown Issue regarding the conda package installation. Therefore, I would like to know how can one solve it, since this error has nothing to do with this Pull Request proposed changes.

Sincerely,

[dopplershift]

@PhilipeRLeal This should be fixed by #1811. Can you rebase on/merge in the current master branch?

[PhilipeRLeal]

Dear @dopplershift, thank you for your reply. I have initiated the rebasing operation on the main branch; nevertheless, I have faced several conflict issues in the process, and I don't know the best approach for handling them.

What do you suggest?

There seem to be two options:

1. apply a "git rebase --skip" operation for each conflict

2. solve each of the conflicts by hand (and there are more than 20 Issues).

The latter alternative is cumbersome, and prone to errors - given that each conflict may involve different modules and submodules.

I hope to hear from you soon.

Sincerely,

[PhilipeRLeal]

I would like to highlight here that the Test suit errors seem to be related to other Issues not relative to this Pull Request.

Most of the error messages are related to the "lib/cartopy/tests/mpl/test_mpl_integration.py", which has nothing to do with this Pull Request.

Can anyone confirm that?

Sincerely,

[dopplershift]

Did you run some kind of tool on the whole source directory? There are a ton of whitespace changes to unrelated files. These changes need to be removed from this PR so we can easily review the relevant changes.

[PhilipeRLeal]

Dear @dopplershift , thank you for your suggestions.

I have updated the whole PR so to solve the errors that were being generated. Nevertheless, as one may notice, there is still a problem with the C.I., and it is due to some inconsistencies of third-party Pull Requests; therefore, completely independent of the changes here proposed.

What do you (or any other member of the developer team) suggest to do now? Can this PR be considered for effectively being merged into Cartopy's master branch?

Let me know if I can be of any assistance.

Sincerely,

[QuLogic]

I have not looked into this deeply because there appear to be several changes that are reverting things that should not be reverted and it is difficult to say what was intentionally changed here. This will need to be sorted out first, and then rebased so that there are no conflicts.

[PhilipeRLeal]

#1863 presented another solution for the Issue of this PR