Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Equal area markers#16891

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
brunobeltran wants to merge7 commits intomatplotlib:main
base:main
Choose a base branch
Loading
frombrunobeltran:equal_area_markers

Conversation

brunobeltran
Copy link
Contributor

@brunobeltranbrunobeltran commentedMar 23, 2020
edited
Loading

PR Summary

WIP. A proposal toresolve#15703.

In short, I propose adding one new parameter toMarkerStyle.__init__, and expanding the options currently available fornormalization (copied from the proposed docstring ofMarkerStyle.__init__):

        normalization : str, optional, default: "classic"            The normalization of the marker size. Can take several values:            *'classic'*, being the default, makes sure custom marker paths are            normalized to fit within a unit-square by affine scaling (but            leaves built-in markers as-is).            *'bbox-width'*, ensure marker path fits in the unit square.            *'area'*, rescale so the marker path has unit "signed_area".            *'bbox-area'*, rescale so that the marker path's bbox has unit            area.            *'none'*, in which case no scaling is performed on the marker path.        centering : str, optional, default: "classic"            The centering of the marker. Can take several values:            *'none'*, being the default, does not translate the marker path.            The origin in path coordinates is the marker center in this case.            *'bbox'*, translates the marker path so that its bbox's center is            at the origin.            *'center-of-mass'*, translates the marker path so that its center            of mass it as the origin. See Path.center_of_mass for details.

Goals

A "complete" custom marker system, that allows the user to choose between several options for marker normalization and centering as is most relevant to their particular plotting goals.

Implementation

The marker customization options will require the following new functionality

  1. Path.get_exact_extents (a bugfix of_path.get_extents does not correctly handle bezier curves #16830, implemented inCorrectly compute path extents #16832)
  2. Path.length (Path length #16888)
  3. Path.signed_area (Compute area of path #16859)
  4. Path.center_of_mass (Path center of mass #16889)

Any reasonable implementation of the above three methods will require

  1. BezierSegment.arc_length (Path length #16888)
  2. BezierSegment.arc_area (Compute area of path #16859)
  3. BezierSegment.center_of_mass (Path center of mass #16889)

EachPath function will simply accumulate the values for itsBezierSegment counterpart to compute the values for the total path (ourPath's right now are simply collections ofBezierSegments, even though they're not stored that way explicitly).

Unfortunately, for methods ofPath to call methods ofBezierSegment, some slight re-arrangements are required to prevent circular import issues (#16812).

Finally, in order to iterate easily over theBezierSegment's that make up the path (to do this accumulation), I'll need to implement

  1. Path.iter_bezier (Bezier/Path API Cleanup: fix circular import issue #16812)

Usingiter_segments directly instead requires a HUGE amount repeated code. Incorporating the proposed.iter_bezier functionality intoiter_segments leads to an absolutely unwieldy amount of bloat initer_segments, so the real question I think is just whether this should be added to the public API (it seems like avery good candidate to me, as it produces MUCH cleaner user code than usingiter_segments for any code I've had to write, and presents amuch more simple model of the underlyingPath).

Roadmap

#16812 (*) <-#16832 (*) <-#16859 (*) <-#16888 <-#16889 (*) <- (This PR)

"<-" means "depends on", and "(*)" marks PRs whose implementations are complete and fully ready for review.

PR Checklist

  • Has Pytest style unit tests
  • Code isFlake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

mwaskom reacted with thumbs up emoji
@jklymak
Copy link
Member

Can we use this PR to talk about the proposed API and the need for it? There are so many other issues and PRs that I am having trouble following what has been said where.

I'd really appreciate a few examples as to what this new feature is supposed to look like to the end user, and a bit more discussion as to whether this is worth the complexity. It seems there is one use case where its "lets let the user normalize the unit size of markers given a limited set of options" all the way to "lets let the user arbitrarily transform markers using the full transform stack". Perhaps these are not incompatible, but it would be nice to see what this will look like by the end of all these PRs.

Right now the only example I've seen in any of the PRs is the quite esoteric desire to lay out pies charts usingscatter, which is pretty gee-whiz, but doesn't strike me as a good argument for any of this functionality. Are there examples that show the power of this approach for actual scatter plots?

@brunobeltran
Copy link
ContributorAuthor

brunobeltran commentedMar 24, 2020
edited
Loading

@jklymak absolutely! Opened this PR to centralize discussion since, as you say, it was pretty fractured before.

I'll let@ImportanceOfBeingErnest speak for the use cases for custom paths for a marker, but we already allow custom paths for markers, that is existing API.

This PR is not about allowing custom markers, but allowing us to normalize markers so that they look perceptually uniform (#15703).

Since we already allow custom markers, any marker normalization code has to be able to perform its duties when passed an arbitrary path. Thankfully, the formulas for the area/extents/etc of an arbitrary path are actually quite simple (thanks to the fact that ourPath objects are just a list of bezier curves), so this PR solves the normalization problem by just implementing the code to compute the area/bbox area/etc of an arbitrary path.

Is the issue that you don't see a use-case for custom markers? We'd have to remove existing 3.2.x API to get rid of them. (Although the changes in this PR would go in more or less as-is regardless).

@ImportanceOfBeingErnest
Copy link
Member

There is a section "Why is this useful?" in my PR#16773.

@mwaskom
Copy link

Just found this issue and wanted to offer my encouragement. It looks like it's going to take substantial effort, but I think it would be a really big improvement.

@tacaswelltacaswell modified the milestones:v3.3.0,v3.4.0May 23, 2020
@jklymakjklymak marked this pull request as draftJuly 23, 2020 03:19
@QuLogicQuLogic modified the milestones:v3.4.0,v3.5.0Jan 21, 2021
@QuLogicQuLogic modified the milestones:v3.5.0,v3.6.0Aug 18, 2021
@timhoffmtimhoffm modified the milestones:v3.6.0,unassignedApr 30, 2022
@story645story645 modified the milestones:unassigned,needs sortingOct 6, 2022
@github-actions
Copy link

Since this Pull Request has not been updated in 60 days, it has been marked "inactive." This does not mean that it will be closed, though it may be moved to a "Draft" state. This helps maintainers prioritize their reviewing efforts. You can pick the PR back up anytime - please ping us if you need a review or guidance to move the PR forward! If you do not plan on continuing the work, please let us know so that we can either find someone to take the PR over, or close it.

@github-actionsgithub-actionsbot added the status: inactiveMarked by the “Stale” Github Action labelJul 14, 2023
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Projects
None yet
Milestone
future releases
Development

Successfully merging this pull request may close these issues.

Sizes of different markers are not perceptually uniform
8 participants
@brunobeltran@jklymak@ImportanceOfBeingErnest@mwaskom@tacaswell@QuLogic@story645@timhoffm

[8]ページ先頭

©2009-2025 Movatter.jp