Skip to content

[MRG] Optimize 3D display (continued) #11577

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.

Sign up for GitHub

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

Closed
rth wants to merge 37 commits into from
Closed

[MRG] Optimize 3D display (continued) #11577

Changes from 1 commit
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
2a75026
Optimize 3D display (#6085)
AlexandreAbraham Jul 5, 2018
b168bed
Remove irrelevant changes
rth Jul 5, 2018
a3c331d
Fix undefined variables
rth Jul 5, 2018
a5c2bd5
Check diff with respect to the original PR
rth Jul 5, 2018
e955f56
Address some review comments
rth Jul 5, 2018
e227b50
Make _juggle_axes_vec and _rotate_axes_vec private
rth Jul 5, 2018
ee53be5
Address review comments
rth Jul 14, 2018
8244c07
Make TestVoxels.test_rgb_data pass
rth Jul 14, 2018
677e79b
Review comment - use np.split + np.cumsum
rth Jul 14, 2018
d095d26
Fix broadcasting
rth Jul 14, 2018
005f6fb
PEP8
rth Jul 15, 2018
f74f3d7
Better debug message
rth Jul 15, 2018
dda0f82
Merge branch 'master' into optimize_3d
rth Jul 15, 2018
1c1e4a1
Fix for numpy 1.10
rth Jul 15, 2018
7a5c50d
Merge branch 'optimize_3d' of github.com:rth/matplotlib into optimize_3d
rth Jul 15, 2018
7c2c22e
Ignore flake8 exception and code clean-up
rth Jul 15, 2018
b81c118
Fix sphinx build
rth Jul 15, 2018
822f4f7
More review comments
rth Jul 15, 2018
004fff2
Fix PEP8
rth Jul 15, 2018
f7795e0
Address part of Eric's comments
rth Jul 16, 2018
da7f72c
Fix anntzer's comments
rth Jul 16, 2018
17657a4
Address Benjamin's review comments
rth Jul 18, 2018
e43b930
Minor fixes
rth Jul 22, 2018
7211708
Removing _juggle_axes_vec and _rotate_axes_vec
rth Jul 22, 2018
5bfa368
Remove flake8 E231 ignore for art3d.py
rth Jul 22, 2018
5faaa0c
Addressed more eric-weiser's comments
rth Jul 23, 2018
7631cfe
Convert _seg_sizes to array
rth Jul 23, 2018
ad0e783
Handle empty sequences
rth Jul 29, 2018
98d1161
Address review comments
rth Aug 18, 2018
1696d52
Experimentally revert get_colors to the previous behaviour
rth Aug 18, 2018
867819f
Merge branch 'master' into optimize_3d
rth Sep 20, 2018
92daa11
Merge branch 'master' into optimize_3d
rth Dec 17, 2018
a268fc7
Fix merge issue
rth Dec 17, 2018
946b9bc
Fix more merge conflicts
rth Dec 17, 2018
ff18f5f
Lint
rth Dec 18, 2018
76d5dc7
Fix one test failure
rth Dec 18, 2018
3e230c7
Address comments
rth Dec 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix for numpy 1.10
  • Loading branch information
@rth
rth committed Jul 15, 2018
commit 1c1e4a189f0685cad0fb604bdfab418fa0757fdc
39 changes: 29 additions & 10 deletions lib/mpl_toolkits/mplot3d/art3d.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from matplotlib import (
artist, cbook, colors as mcolors, lines, text as mtext, path as mpath)
from matplotlib.collections import (
LineCollection, PolyCollection, PatchCollection,
Collection, LineCollection, PolyCollection, PatchCollection,
PathCollection)
from matplotlib.colors import Normalize
from matplotlib.patches import Patch
Expand Down Expand Up @@ -72,6 +72,26 @@ def get_dir_vector(zdir):
raise ValueError("'x', 'y', 'z', None or vector of length 3 expected")


def _array_split(arr, indices_or_sections, remove_empty=False):
"""Fix numpy.split to preserve the dimension of empty subarrays.
"""

arr_chunks = np.split(arr, indices_or_sections)

if arr_chunks[-1].size == 0:

if not remove_empty:
# Preserve the 2D dimensionality the last chunk that can be empty

@eric-wieser eric-wieser Jul 16, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

missing "of"

# (numpy <=1.10 replaces empty chunks by a 1D empty array)
# TODO: The following can be removed when
# support for numpy <=1.10 is dropped.
arr_chunks[-1] = np.empty(shape=(0, arr.shape[1]), dtype=arr.dtype)
else:
del arr_chunks[-1]

return arr_chunks


class Text3D(mtext.Text):
"""
Text object with 3D position and direction.
Expand Down Expand Up @@ -245,8 +265,8 @@ def set_segments(self, segments):
self._segments3d_data[:, 3] = 1

@rth rth Jul 15, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

#6085 (comment) by @eric-wieser

This would be marginally better as

self._segments3d_data  = np.empty((n_segments, 4))
self._segments3d_data[:,:3] = segments
self._segments3d_data[:,3] = 1

Done.

Also, are you sure you mean "quaternion" and not "homogenous coordinate"?

Not sure, but I removed the "quaternion" term as suggested.


# For coveniency, store a view of the array in the original shape

@eric-wieser eric-wieser Jul 16, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

typo in "convenience`

self._segments3d = np.split(self._segments3d_data[:, :3],
np.cumsum(self._seg_sizes))
self._segments3d = _array_split(self._segments3d_data[:, :3],
np.cumsum(self._seg_sizes))

@rth rth Jul 15, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

#6085 (comment) by @eric-wieser

I think this loop is np.split in some form

Definitely, changed.

else:
self._seg_sizes = np.array([])

Expand All @@ -259,8 +279,8 @@ def do_3d_projection(self, renderer):
if len(self._segments3d) == 0:
return 1e9

@eric-wieser eric-wieser Jul 29, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe the old fixme should stay here, with a comment explaining its for back-compat. I'd be happier with a change to float('inf') here in some future patch.

xys = proj3d.proj_transform_vec(self._segments3d_data.T, renderer.M).T

@WeatherGod WeatherGod Jul 18, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

no need for the second transpose. All remaining uses of xys are indexings, which you can transpose yourself.

segments_2d = np.split(xys[:, :2],
np.cumsum(self._seg_sizes))
segments_2d = _array_split(xys[:, :2],
np.cumsum(self._seg_sizes))
LineCollection.set_segments(self, segments_2d)
minz = np.min(xys[:, 2])
return minz
Expand Down Expand Up @@ -622,11 +642,10 @@ def do_3d_projection(self, renderer):
self._facecolors3d = self._facecolors

xys = proj3d.proj_transform_vec(self._vec, renderer.M).T
xyzlist = []
cum_s = 0
for s in self._seg_sizes:
xyzlist.append(xys[cum_s:cum_s + s, :3])
cum_s += s

xyzlist = _array_split(xys[:, :3],
np.cumsum(self._seg_sizes),
remove_empty=True)

@eric-wieser eric-wieser Jul 29, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Instead of remove_empty here, you could just pass np.cumsum(self._seg_sizes[:-1])


# This extra fuss is to re-order face / edge colors
cface = self._facecolors3d
Expand Down