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: array_view construction for empty arrays #5106
Changes from 6 commits
e6b1745
724cb9b
2f29bdf
c0e09d5
536d175
3965858
2f5a633
123deb5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -80,9 +80,7 @@ class Collection(artist.Artist, cm.ScalarMappable): | |
# _offsets must be a Nx2 array! | ||
_offsets.shape = (0, 2) | ||
_transOffset = transforms.IdentityTransform() | ||
_transforms = [] | ||
|
||
|
||
_transforms = np.empty((0, 3, 3)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since you've touched it, would you mind adding a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where is this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think @pelson just means to add a normal comment explaining what the expected shape of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed it does: http://sphinx-doc.org/ext/autodoc.html#directive-autoattribute |
||
|
||
def __init__(self, | ||
edgecolors=None, | ||
|
@@ -1515,7 +1513,7 @@ def __init__(self, widths, heights, angles, units='points', **kwargs): | |
self._angles = np.asarray(angles).ravel() * (np.pi / 180.0) | ||
self._units = units | ||
self.set_transform(transforms.IdentityTransform()) | ||
self._transforms = [] | ||
self._transforms = np.empty((0, 3, 3)) | ||
self._paths = [mpath.Path.unit_circle()] | ||
|
||
def _set_transforms(self): | ||
|
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.
Do we want to make this a public function? We now have
_check_1d()
,_reshape_2D
, and this newensure_3d
(notice the inconsistency with the case of "d/D").Why don't we make this private, and make a new issue to unify the logic across these three functions later (if possible)?
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 would have thought that an underscore in the beginning means a function that is only called from within cbook, but clearly some underscored functions are called from elsewhere. What is the intended meaning?
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.
https://www.python.org/dev/peps/pep-0008/#id29
It means that it is to be treated as being for internal use only. This
gives developers the freedom to modify the API of such modules or functions
without warning or deprecation cycles. As noted later in PEP8, it is easier
to start with something as private and then make it public later, as
opposed to releasing it as public and then modifying it later.
On Mon, Sep 28, 2015 at 10:23 AM, Jouni K. Seppänen <
notifications@github.com> wrote:
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.
PEP8 doesn't really say what the boundaries of internal use are, but I guess here it means internal to matplotlib. Ok, I'll change this.