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

Make plot functions work with merged glaciers #726

Merged
merged 5 commits into from Mar 29, 2019

Conversation

Projects
None yet
3 participants
@matthiasdusch
Copy link
Member

commented Mar 29, 2019

Need this for a merged glacier tutorial.
Also added the workflow task to the API documentation.

  • what's new entry
@pep8speaks

This comment has been minimized.

Copy link

commented Mar 29, 2019

Hello @matthiasdusch! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-03-29 14:28:26 UTC
@fmaussion
Copy link
Member

left a comment

Thanks! Usually I would like to add a test for this (one plot function with multiple glaciers) but given that it's going to become a tutorial I'm OK with leaving it open for now ;-)

Add a line line in "what's new" though

linewidth=0.5)
else:
for l in poly_pix.interiors:
smap.set_geometry(l, crs=crs, color='black', linewidth=0.5)

This comment has been minimized.

Copy link
@fmaussion

fmaussion Mar 29, 2019

Member

Would the following work?

poly_pix = utils.tolist(poly_pix)
for _poly in poly_pix:
    for l in _poly.interiors:
        smap.set_geometry(l, crs=crs, color='black',
                                         linewidth=0.5)

Alternatively:

if not isinstance(poly_pix, shpg.MultiPolygon):
    poly_pix = [poly_pix]

(just to avoid duplication)

This comment has been minimized.

Copy link
@matthiasdusch

matthiasdusch Mar 29, 2019

Author Member

yes, I like that

matthiasdusch added some commits Mar 29, 2019

@matthiasdusch

This comment has been minimized.

Copy link
Member Author

commented Mar 29, 2019

Note for future: This PR only fixes graphics.plot_domain, graphics.plot_centerlines and
graphics_plot_modeloutput_map for merged glaciers. Other plot functions need more work or will never work as they for example need inversion results or catchment indices which are not processed for merged glaciers.

@matthiasdusch matthiasdusch merged commit 50bba97 into OGGM:master Mar 29, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.07%) to 87.589%
Details

@matthiasdusch matthiasdusch deleted the matthiasdusch:merged_graphics branch Mar 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.