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 support for the 'reference' attribute on the vector gm #389

Merged
merged 22 commits into from
Feb 27, 2019

Conversation

scottwittenburg
Copy link
Collaborator

The 'reference' attribute has been available to read/write on
the vector graphics method, but until now has not been implemented.
Now, if it is set, the default behavior of choosing a sensible value
for the arrow length based on the screen space availabe, text size,
etc., will be overridden. Instead the arrow length will simply be
chosen to match the reference attribute. Hence, this feature should
be used with care, as it could result in a vector legend with either
very large or very small reference arrow.

@scottwittenburg
Copy link
Collaborator Author

This addresses part of #386, by supporting the reference attribute of the vector graphics method.

Needs baselines from here.

@scottwittenburg
Copy link
Collaborator Author

@danlipsa Can you take a look at this when you get a chance? My goal was to keep your code implementing the default behavior intact (choosing an appropriate size for the vector legend arrow), while also supporting the reference attribute as an override.

ping @aashish24

The 'reference' attribute has been available to read/write on
the vector graphics method, but until now has not been implemented.
Now, if it is set, the default behavior of choosing a sensible value
for the arrow length based on the screen space availabe, text size,
etc., will be overridden.  Instead the arrow length will simply be
chosen to match the reference attribute.  Hence, this feature should
be used with care, as it could result in a vector legend with either
very large or very small reference arrow.
@scottwittenburg scottwittenburg force-pushed the support-vector-reference-attribute branch from c86a745 to 95cc8ef Compare February 14, 2019 01:46
@coveralls
Copy link

coveralls commented Feb 14, 2019

Coverage Status

Coverage increased (+0.01%) to 70.903% when pulling 48420d1 on support-vector-reference-attribute into 22418d8 on master.

@danlipsa
Copy link
Contributor

+1. Looks good!

Copy link
Contributor

@doutriaux1 doutriaux1 left a comment

Choose a reason for hiding this comment

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

@scottwittenburg shouldn't we update the doc string to explain better what the NNormalize options really mean?

@scottwittenburg
Copy link
Collaborator Author

@doutriaux1 We definitely should, I was hoping that @aashish24 could eventually chime in since he wrote that code. And the commit message you pointed me to last week (or was it the one before) didn't make sense to me.

@doutriaux1
Copy link
Contributor

@danlipsa i limites this pr to mac py3 let's use it to test your work. I tried to up xcode but no luck on that end either

@doutriaux1
Copy link
Contributor

@danlipsa. I would say ssh into this and try to do your manual build here against the python that was pulled. What do you think?

@doutriaux1
Copy link
Contributor

doutriaux1 commented Feb 26, 2019

@danlipsa ok so py36 works but not py37 so i think you'll need to manually build for py37. Sorry about that.

@doutriaux1 doutriaux1 merged commit cdc8c04 into master Feb 27, 2019
@doutriaux1 doutriaux1 deleted the support-vector-reference-attribute branch February 27, 2019 00:34
@doutriaux1 doutriaux1 added this to the 8.1 milestone Feb 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants