-
-
Notifications
You must be signed in to change notification settings - Fork 402
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
Ellipse parameter now supports major/minor axis lengths #1509
Merged
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
ef12996
Ellipse parameter now supports major/minor axis lengths
jlstevens 880e5d9
Fixed typo in docstring
jbednar 90bda0c
Simplified Ellipse to support a straightforward width parameter
jlstevens f3f24b0
Fixed incorrect docstring
jlstevens a539a37
Updated docstring and restricted aspect to the height-only spec
jlstevens 8e0a7c1
Fixed clone method for BaseShapes
jlstevens 3b02880
Further fixes and improvements to Ellipse
jlstevens 01fd33a
Made the Box element's API consistent with Ellipse
jlstevens 86c11fd
Fixed outdated docstrings
jlstevens 85ee24d
Updated Ellipse element notebooks
jlstevens 3fefa32
Updated Box and Ellipse element notebooks
jlstevens e2df3ee
Added seven unit tests of Ellipse and Box
jlstevens 9079666
Made Box element notebooks Python 3 compatible
jlstevens File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
""" | ||
Unit tests of Path types. | ||
""" | ||
import numpy as np | ||
from holoviews import Ellipse, Box, Bounds | ||
from holoviews.element.comparison import ComparisonTestCase | ||
|
||
|
||
class EllipseTests(ComparisonTestCase): | ||
|
||
def setUp(self): | ||
self.pentagon = np.array([[ 0.00000000e+00, 5.00000000e-01], | ||
[ 4.75528258e-01, 1.54508497e-01], | ||
[ 2.93892626e-01, -4.04508497e-01], | ||
[ -2.93892626e-01, -4.04508497e-01], | ||
[ -4.75528258e-01, 1.54508497e-01], | ||
[ -1.22464680e-16, 5.00000000e-01]]) | ||
|
||
self.squashed = np.array([[ 0.00000000e+00, 1.00000000e+00], | ||
[ 4.75528258e-01, 3.09016994e-01], | ||
[ 2.93892626e-01, -8.09016994e-01], | ||
[ -2.93892626e-01, -8.09016994e-01], | ||
[ -4.75528258e-01, 3.09016994e-01], | ||
[ -1.22464680e-16, 1.00000000e+00]]) | ||
|
||
|
||
def test_ellipse_simple_constructor(self): | ||
ellipse = Ellipse(0,0,1, samples=100) | ||
self.assertEqual(len(ellipse.data[0]), 100) | ||
|
||
def test_ellipse_simple_constructor_pentagon(self): | ||
ellipse = Ellipse(0,0,1, samples=6) | ||
self.assertEqual(np.allclose(ellipse.data[0], self.pentagon), True) | ||
|
||
def test_ellipse_tuple_constructor_squashed(self): | ||
ellipse = Ellipse(0,0,(1,2), samples=6) | ||
self.assertEqual(np.allclose(ellipse.data[0], self.squashed), True) | ||
|
||
def test_ellipse_simple_constructor_squashed_aspect(self): | ||
ellipse = Ellipse(0,0,2, aspect=0.5, samples=6) | ||
self.assertEqual(np.allclose(ellipse.data[0], self.squashed), True) | ||
|
||
|
||
class BoxTests(ComparisonTestCase): | ||
|
||
def setUp(self): | ||
self.rotated_square = np.array([[-0.27059805, -0.65328148], | ||
[-0.65328148, 0.27059805], | ||
[ 0.27059805, 0.65328148], | ||
[ 0.65328148, -0.27059805], | ||
[-0.27059805, -0.65328148]]) | ||
|
||
self.rotated_rect = np.array([[-0.73253782, -0.8446232 ], | ||
[-1.11522125, 0.07925633], | ||
[ 0.73253782, 0.8446232 ], | ||
[ 1.11522125, -0.07925633], | ||
[-0.73253782, -0.8446232 ]]) | ||
|
||
def test_box_simple_constructor_rotated(self): | ||
box = Box(0,0,1, orientation=np.pi/8) | ||
self.assertEqual(np.allclose(box.data[0], self.rotated_square), True) | ||
|
||
|
||
def test_box_tuple_constructor_rotated(self): | ||
box = Box(0,0,(1,2), orientation=np.pi/8) | ||
self.assertEqual(np.allclose(box.data[0], self.rotated_rect), True) | ||
|
||
def test_box_aspect_constructor_rotated(self): | ||
box = Box(0,0,1, aspect=2, orientation=np.pi/8) | ||
self.assertEqual(np.allclose(box.data[0], self.rotated_rect), True) |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this necessary? Why would
self.__pos_params
work?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why wouldn't it work? Name mangling due to the double underscore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, instead of
getattr
? In this case it might work...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope just checked, name mangling is still an issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might not work actually, for that reason a single underscore may have been cleaner. There's a bunch of them now though, so up to you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe single underscore would have messed up
Layouts
andAttrTrees
. It was done like this for a reason but I can't quite remember all the details.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I partially remember -
clone
has to usegetattr
as not all elements need the positional parameters declared (most don't).Calling
getattr
will create a path onAttrTrees
resulting in an annoying entry which doesn't happen for double underscored entries. You have to be careful withgetattr
whenAttrTrees
are involved...