Skip to content

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Dec 27, 2018

Rationale

In Matplotlib, there are 4 builtin geo projections: Aitoff, Hammer, Mollweide, and Lambert. They are not quite as flexible as Cartopy, but work for maps at least. We have the latter two, but not the former.

Implications

This adds Aitoff and Hammer. In proj, Hammer support two more parameters: +W and +M. I do not know what these stand for, so I have not added them, but would like to if anyone has any understanding of what they mean.

Copy link
Member

@pelson pelson left a comment

Choose a reason for hiding this comment

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

LGTM once tests passing (haven't looked into the reason they aren't).

@QuLogic
Copy link
Member Author

QuLogic commented Jan 10, 2019

Any thoughts on Hammer parameters?

It seems like 4.9.3 might be buggy with one of these.

@pelson
Copy link
Member

pelson commented Jan 10, 2019

Any thoughts on Hammer parameters?

As in +W and +M? I'm quite content to not provide these until they are needed (in the form of a PR from the person that does).

The test is failing with a lot of warnings, but they may be not an issue? The image that now has all of the projections is (copied from Travis):

full

@greglucas
Copy link
Contributor

Interest in coming back to these projections?

@QuLogic QuLogic changed the title WIP: Add Matplotlib geo projections Add Matplotlib geo projections Dec 24, 2021
@QuLogic
Copy link
Member Author

QuLogic commented Dec 24, 2021

Rebased, but I did not add the other parameters. I'm also not sure if the threshold needs setting nowadays.

@QuLogic
Copy link
Member Author

QuLogic commented Dec 24, 2021

Oops, created the test images with the oldest supported Matplotlib instead of the latest.

Copy link
Contributor

@greglucas greglucas left a comment

Choose a reason for hiding this comment

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

I just had one minor comment, which should probably be a follow-up PR. Looks good to me otherwise.

Comment on lines +2146 to +2147
.. note::
This projection does not handle elliptical globes.
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't look like this note appears anywhere in the docs? It seems like all of the init docstrings are left out currently.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, maybe it was with the change in doc themes? I don't remember what the older one looked like now.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can open up another issue if we want to update the docs later.

@greglucas greglucas merged commit 4f61922 into SciTools:main Sep 11, 2022
@greglucas
Copy link
Contributor

Sorry, I actually meant to merge this in before the release and just forgot.

@QuLogic QuLogic deleted the mpl-geo branch September 11, 2022 08:21
@QuLogic QuLogic added this to the 0.22 milestone Sep 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants