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

Fix list parameters passed to artists #910

Closed
wants to merge 2 commits into from

Conversation

has2k1
Copy link

@has2k1 has2k1 commented Jul 18, 2017

Problem

When artists are being drawn, the geometries outside the viewport
are removed before the drawing. The parameters that correspond to
removed geometries are left unchanged and so they are ill-matched
with the remaining geometries.

Solution

Keep track of which geometries are removed, and remove the
corresponding elements in the parameter values.

Fixes #580

Also

@has2k1
Copy link
Author

has2k1 commented Jul 18, 2017

One test failure.

The file header at lib/cartopy/mpl/feature_artist.py is out of date. The last commit was in 2017, but the copyright states it was 2016.

Can I update the date at the top of the affected file?

@QuLogic
Copy link
Member

QuLogic commented Jul 18, 2017

Can I update the date at the top of the affected file?

You should if you changed it.

@QuLogic
Copy link
Member

QuLogic commented Jul 18, 2017

Looks like you'll also need to sign the CLA as well.

@QuLogic QuLogic mentioned this pull request Aug 4, 2017
@pelson pelson added the Blocked: CLA needed See https://scitools.org.uk. Submit the form at: https://scitools.org.uk/cla/v4/form label Aug 17, 2017
@has2k1
Copy link
Author

has2k1 commented Aug 17, 2017

I signed the CLA form and emailed a copy to the address and it's been a while without a reply.

@QuLogic
Copy link
Member

QuLogic commented Aug 18, 2017

Ping @marqh, I think?

@pelson pelson removed the Blocked: CLA needed See https://scitools.org.uk. Submit the form at: https://scitools.org.uk/cla/v4/form label Oct 26, 2017
@has2k1
Copy link
Author

has2k1 commented Oct 26, 2017

It would be great to have tests for all feature types, but it does not look easy. Any ideas on to generate good tests? For the ShapelyFeature the Katrina example now touches on code introduced in this PR (it is a "test" of sorts).

Rebased and travis did not like it. Maybe it now requires #926.

@dopplershift
Copy link
Contributor

Yeah, #926 fixes a lot of testing issues. @pelson ?

@has2k1 has2k1 changed the title WIP: Fix list parameters passed to artists Fix list parameters passed to artists Jan 5, 2018
**Problem**

When artists are being drawn, the geometries outside the viewport
are removed before the drawing. The parameters that correspond to
removed geometries are left unchanged and so they are ill-matched
with the remaining geometries.

**Solution**

Keep track of which geometries are removed, and remove the
corresponding elements in the parameter values.

Fixes SciTools#580
@pelson
Copy link
Member

pelson commented Feb 21, 2018

This has been a really valuable PR that has ultimately led to #1019, which is my preferred direction for finer control of the style through add_geometries (and ```add_feature``).

As such, I don't think we need this PR open any further.
The relevant thread of conversation is #1019 (comment). As discussed there, there is the potential for an extension to create a GeometryCollection, but even that is probably best served in a new PR.

Thanks again for your input on this @has2k1 - it has been super useful, and I'm really pleased with the outcome, which I consider to be an extremely powerful interfaces for all sorts of purposed.

👍

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.

4 participants