Skip to content

Conversation

pmarshwx
Copy link

This fixes the scenario where plotting fails when a user passes latlon=False to a 2d plotting function. The error arises because latlon is only removed from the keyword arguments if it is True. If it was false, it was never removed from the keyword arguments and then passed on to matplotlib, which failed on an unsupported keyword argument.

The fix simply reworks the keyword checking logic to ensure that if latlon is present in the keyword arguments it is removed before matplotlib is called.

# convert lat/lon coords to map projection coords.
x, y = self(x,y)
if 'latlon' in kwargs:
if kwargs['latlon']:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A simpler way to do this replaces the two "if" statements and the "del kwargs['latlon']" with:

            if kwargs.pop("latlon", False):

In other words, in the original, replace "kwargs.get" with "kwargs.pop", and remove the "del" statement.

@pmarshwx
Copy link
Author

I hadn't thought of that. It is a cleaner approach to the problem.

Being a relative newcomer to this functionality of git, what's the best way to make this change?

@efiring
Copy link
Member

efiring commented Aug 20, 2012

On 2012/08/19 12:12 PM, Patrick Marsh wrote:

I hadn't thought of that. It is a cleaner approach to the problem.

Being a relative newcomer to this functionality of git, what's the best
way to make this change?

I think what you want is a single clean commit that makes the desired
overall change. I will assume your repo on your local machine has
remotes "origin", which is your own gitub clone of basemap, and
"upstream", which is the master basemap repo. Then do something like
this in your local repo:

git checkout master
git pull upstream master
git checkout -b latlonbugfix2 master

Now make the fix there as you actually want it, build/install/test it,
and commit it. Then do a forced push from your new latlonbugfix2 branch
to your github latlonbugfix branch:

git push -f origin latlonbugfix2:latlonbugfix

At this point your original pull request should have in it only the new,
revised commit, so if/when Jeff merges it, everything is nice and clean.
At any point you can delete your original local branch like this:

git branch -D latlonbugfix

since you no longer need it. The -D is needed rather than -d because the
branch has not been merged. When your pull request is merged, then you
can again

git checkout master
git pull upstream master
git branch -d latlonbugfix2

since you don't need it any more. You can (but don't have to) delete it
from your github branch with

git push origin :latlonbugfix

This slightly obscure syntax means something like "delete the remote
branch by pushing to it from nowhere."

The forced push in the earlier step is an example of rewriting history,
and, like rebasing, it is OK provided it is occurring in a repo that no
one is cloning. This is generally the case for one's own github forks.

Eric

…ere passed to matplotlib causing the plot to fail.
@pmarshwx
Copy link
Author

Thank you very much, Eric for taking the time and for writing that explanation. It was exactly what I was looking to know how to do. This will greatly enhance my ability to contribute to the MPL ecosystem.

The new (single) commit has been pushed. Thanks again.

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

Successfully merging this pull request may close these issues.

3 participants