Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add all 60 zones of the UTM projection to the docs projection list #1103

Merged
merged 14 commits into from
Sep 27, 2018

Conversation

kaedonkers
Copy link
Member

@kaedonkers kaedonkers commented Aug 9, 2018

Rationale

In the projection list of the documentation the UTM projection is depicted using only one zone. It would be more informative to include all 60 zones of the projection in the list, side by side (similar to the example added in PR #954).

Implications

Rewrites make_projection.py to include options for multiple subplots as well as single plots. Multiple subplots have been utilised for both PlateCarree and UTM projections.
Closes issue #986.

@kaedonkers
Copy link
Member Author

@pelson could you review this please?


def plate_carree_plot(i, nplots):
central_longitude = 0 if i == 1 else 180
ax = fig.add_subplot(1, nplots-1, i,

Choose a reason for hiding this comment

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

F821 undefined name 'fig'

Copy link
Member Author

Choose a reason for hiding this comment

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

Needed for generating plotting code.

central_longitude = 0 if i == 1 else 180
ax = fig.add_subplot(1, nplots-1, i,
projection=ccrs.PlateCarree(
central_longitude=central_longitude))

Choose a reason for hiding this comment

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

E122 continuation line missing indentation or outdented



def utm_plot(i, nplots):
ax = fig.add_subplot(1, nplots-1, i,

Choose a reason for hiding this comment

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

F821 undefined name 'fig'

for instance_args in SPECIAL_CASES.get(prj, [{}]):
prj_inst = prj(**instance_args)

if prj not in MULTI_PLOT_CASES:

Choose a reason for hiding this comment

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

E303 too many blank lines (2)

# Generate plotting code
code = textwrap.dedent("""
.. plot::

Choose a reason for hiding this comment

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

W293 blank line contains whitespace


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

Choose a reason for hiding this comment

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

W293 blank line contains whitespace


code = textwrap.dedent("""
.. plot::

Choose a reason for hiding this comment

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

W293 blank line contains whitespace

import cartopy.crs as ccrs

fig = plt.figure(figsize=(10, 3))

Choose a reason for hiding this comment

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

W293 blank line contains whitespace

fig = plt.figure(figsize=(10, 3))

{func_code}

Choose a reason for hiding this comment

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

W293 blank line contains whitespace

\n""".format(width=width, proj_constructor=instance_creation_code,
coastline_resolution=COASTLINE_RESOLUTION.get(prj, '110m'))
""").format(nplots=nplots+1,
func_code=func_code, func_name=func.__name__)

Choose a reason for hiding this comment

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

E128 continuation line under-indented for visual indent


def plate_carree_plot(i, nplots):
central_longitude = 0 if i == 1 else 180
ax = fig.add_subplot(1, nplots-1, i,

Choose a reason for hiding this comment

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

F821 undefined name 'fig'

# Generate plotting code
code = textwrap.dedent("""
.. plot::

Choose a reason for hiding this comment

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

W293 blank line contains whitespace


def plate_carree_plot(i, nplots):
central_longitude = 0 if i == 1 else 180
ax = fig.add_subplot(1, nplots-1, i,

Choose a reason for hiding this comment

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

F821 undefined name 'fig'

Copy link
Member

@QuLogic QuLogic left a comment

Choose a reason for hiding this comment

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

This is mostly better, but there's something a bit off with the width. It seems like the existing plots were never too wide, but these new figures are too wide to fit and are shrunk down a bit, producing a slightly poorer looking figure.

Also, some minor stylistic issues below.

@@ -19,26 +19,53 @@

import os

Copy link
Member

Choose a reason for hiding this comment

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

Don't need this blank line.

@@ -19,26 +19,53 @@

import os

import inspect

Copy link
Member

Choose a reason for hiding this comment

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

Also unnecessary blank line.


def plate_carree_plot(i, nplots, fig):
central_longitude = 0 if i == 1 else 180
ax = fig.add_subplot(1, nplots-1, i,
Copy link
Member

Choose a reason for hiding this comment

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

This -1 is confusing; you should +1 on the i instead.


\n""".format(width=width, proj_constructor=instance_creation_code,
coastline_resolution=COASTLINE_RESOLUTION.get(prj, '110m'))
""").format(nplots=nplots+1,
Copy link
Member

Choose a reason for hiding this comment

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

And then remove this +1.

@@ -67,7 +62,7 @@ AzimuthalEquidistant

plt.figure(figsize=(3, 3))
ax = plt.axes(projection=ccrs.AzimuthalEquidistant(
central_latitude=90))
central_latitude=90))
Copy link
Member

Choose a reason for hiding this comment

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

There's too much indent to this now.

@kaedonkers
Copy link
Member Author

kaedonkers commented Aug 10, 2018

Good point about the width @QuLogic. I've returned PlateCarree to being vertically stacked.
The UTM projection is now the widest figure (figsize=(10, 3)), but this is only marginally more than the previous widest projection LambertCylindrical (figsize=(9.4248, 3)).
If the width of the UTM projection is still too much I could shave it down to 9.5.

@kaedonkers
Copy link
Member Author

@pelson @QuLogic Does this look ready to merge?

@QuLogic
Copy link
Member

QuLogic commented Sep 27, 2018

I was going to suggest fig.subplots_adjust(wspace=0), but that doesn't seem to have any effect at the figure size/dpi that would fit in the docs.

@QuLogic QuLogic merged commit e9df849 into SciTools:master Sep 27, 2018
@QuLogic QuLogic added this to the 0.17 milestone Sep 27, 2018
@QuLogic
Copy link
Member

QuLogic commented Sep 27, 2018

Closes issue #956.

I'm not sure how this is related since this PR is just documentation, so I haven't closed that issue yet.

@kaedonkers
Copy link
Member Author

Thanks @QuLogic!

Oops, right you are. It closes #986, not #956. I'll go ahead and close the correct issue.

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