Addition of new functionalities for Cartopy.gridliner #1693
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
There are 311 errors:
|
SciTools-assistant
added
the
Blocked: CLA needed
|
There are 311 errors:
|
|
I state that I have now signed the Contributor's Licence Agreement (CLA) document. |
stickler-ci
reviewed
Dec 16, 2020
|
Thanks for the PR @PhilipeRLeal! There are a lot of extraneous changes here to what you're trying to accomplish which makes it hard to review (i.e. linting changes in unrelated files). It looks like there are two things you're trying to put in, a new formatter and a new scalebar? I'd suggest making these two separate smaller PRs as I think they are two distinct features. Can you rebase and squash into one or two meaningful commits to make the review easier? You have two tests failing, one license file header and one in the new test you added test_scalebar. See here for those failures: https://travis-ci.org/github/SciTools/cartopy/jobs/750254210 The SciTools CLA checker will automatically get updated when someone has the time to review your submitted CLA and they'll automatically remove it I think. |
Dear GregLucas, thank you for your suggestions. This is my first commit to a open source library as cartopy. I still don't know what "rebase" and "squash" mean. By any chance, "rebase" would be changing the pull request's Title, or hash code (i.e.: #1693)? If so, how can I do it? Nevertheless, you are right about the things I am trying to put in this pull request. There are two changes that I am proposing. First, I am trying to add a new customization option for the gridlines. This problem is presented in a comment from the stackoverflow (see here). At the current time, cartopy only supports a fixed set of geographical units for the xticklabels and yticklabels (i.e.: "W" for west coordinates, "E" for east coordinates, "S" for south coordinates, "N" for north coordinates), with no easy way to change them to a user's defined version. Also, there is no easy way to convert the decimal symbol of the coordinates (i.e.: dot "."). For users like me who come from Latin countries, there is a great demand for latin symbols, therefore, instead of using [W,E,S,N and dot"."], one should use the following respectively [O, L, S, N and ","]. Here is a relation of the provided symbols: O (from Oeste) == West I state here that all prior symbols [W,E,S,N and dot"."] have been kept as they were (in international format). Therefore, no changes have been made to the common cartopy's gridline usage. This new Pull request only allows the user to easily change those symbols into any user's defined string/symbol. ---------------------------------------------------------------------------------------------------------------Lastly, I am also proposing a method to draw a scalebar to cartopy's geoaxes. This addition would be directly callable from the geoaxes, and it should be capable of evaluating metric distances (i.e.: meters and km units) in axesTransAxes units. Prior to cartopy's development, the mpl.basemap had something similar, which allowed easy evaluation and plotting of a scalebar into one's geographical Map. Nevertheless, despite continuous development in cartopy's, this functionality has not been formally proposed. This scalebar method should also be prone to user's modifications as, for example, setting the decimal separator and unit symbology (i.g.: "km", "mi", etc.). ---------------------------------------------------------------------------------------------------------------Both new additions have been tested, and are currently running in my computer. Nevertheless, I use Windows, and therefore, I can't properly create a build for the package prior a commit due to GEOS interface. That is one of the main issues that I am struggling with in this pull request. I believe that this new pull request can truly help Python's community, specially cartopy's. I put myself at your disposal, and I hope to hear from you soon. Sincerely, Philipe Riskalla Leal |
PhilipeRLeal
pushed a commit
to PhilipeRLeal/cartopy
that referenced
this pull request
Dec 21, 2020
…onalities here provided. The prior developers branch had several commits which were aggregated into this single 1st commit. Add a warning and comments This is the commit that antecedes all added new commits in respect to the pull request number SciTools#1693 from: SciTools#1693
SciTools-assistant
removed
the
Blocked: CLA needed
|
Welcome to the open source community :) For rebasing and squashing I was referring specifically to The whole reason for that is right now I see 97 commits and 100+ files changed, so git needs to know what it is merging against to properly see new vs old commits. The reason for requesting two separate PRs for this work is that they are distinct entities and maybe one will be accepted and another not, or one needs lots of changes but the other doesn't. It is just easier to work on smaller chunks at a time. |
|
Could not review pull request. It may be too large, or contain no reviewable changes. |
PhilipeRLeal
pushed a commit
to PhilipeRLeal/cartopy
that referenced
this pull request
Dec 22, 2020
…onalities here provided. The prior developers branch had several commits which were aggregated into this single 1st commit. Add a warning and comments This is the commit that antecedes all added new commits in respect to the pull request number SciTools#1693 from: SciTools#1693
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Dear all,
I would like to make a pull request in cartopy in order to add new functionalities for cartopy.
Rationale
These new assets have been derived from my personal struggle with cartopy's gridline and the continuous migration from the past basemap towards cartopy.
In this new branch, the gridline class has gained new assets for easier usage, especially regarding ticklabel_settings.
The list of the new functionalities related to gridline ticklabel settings:
Changes in respect to basemap->cartopy migration:
There has been a lot of discussion regarding scalebars (rulerbars) for geographical data in python, especially regaring cartopy:
Therefore, in this branch, I also developed a scalebar module, from which the user can draw a geographical rulerbar much similar to the ones observed in Geographical Information Systems (GIS). At the moment, I have set the module inside the cartopy.mpl subdirectory. That way, the manipulation of the geographical information becomes more organized in a same subpackage.
One aspect that may cause some constraints is that the scalebar module requires the geopy package. In future versions, its dependency may be broken by altering the geopy functions applied in the scalebar module.
Usage example the scalebar module
Implications
addition of a dependency of geopy package solely for the scalebar module.