jupyter-widgets / pythreejs

A Jupyter - Three.js bridge
https://pythreejs.readthedocs.io
Other
943 stars 188 forks source link

Fixing the error that arise when using uneven width and height #360

Closed zeyuyun1 closed 2 years ago

zeyuyun1 commented 3 years ago

The current code for SurfaceGeometry works only when width_segments==height_segments. For example,

surf_g = SurfaceGeometry(z=[0]*100, 
                         width_segments=nx - 1,
                         height_segments=ny - 1)

will only run when nx == ny.

Error arise when we have:

nx = 10
ny = 15
surf_g = SurfaceGeometry(z=[0]*150, 
                         width_segments=nx - 1,
                         height_segments=ny - 1)

ValueError: all the input array dimensions for the concatenation axis must match exactly, but along dimension 0, the array at index 0 has size 15 and the array at index 2 has size 10

This is because in line 81 z = np.array(self.z).reshape((nx, ny)) the reshaped dimension is reversed.

The proposed change fixed this error without interrupting any other part.

vidartf commented 2 years ago

Thanks for this contribution! I tested the fix locally using the code in examples/Examples.ipynb, and while this fixes the positions, it does seem like the indices are not correct in the nx != ny case. I'll dig some more on my side to see if I can spot the error quickly.

codecov-commenter commented 2 years ago

Codecov Report

Merging #360 (7eea4c0) into master (150ff1c) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #360   +/-   ##
=======================================
  Coverage   70.86%   70.86%           
=======================================
  Files          23       23           
  Lines         834      834           
=======================================
  Hits          591      591           
  Misses        243      243           
Impacted Files Coverage Δ
pythreejs/pythreejs.py 70.70% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 150ff1c...7eea4c0. Read the comment docs.