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

Support empty ArraySequence in saving (for empty vtk) #593

Merged
merged 7 commits into from Jun 10, 2022

Conversation

frheault
Copy link
Contributor

Added a simple check to avoid the np.stack if an array is empty.

The diff looks weird, but it's simply a new if to avoid the coloring part if there is no streamlines

@codecov
Copy link

codecov bot commented May 27, 2022

Codecov Report

Merging #593 (fa66ca3) into master (6fe15a6) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #593   +/-   ##
=======================================
  Coverage   87.94%   87.94%           
=======================================
  Files          62       62           
  Lines       13099    13120   +21     
  Branches     1316     1320    +4     
=======================================
+ Hits        11520    11539   +19     
- Misses       1202     1203    +1     
- Partials      377      378    +1     
Impacted Files Coverage Δ
fury/fury/utils.py 91.47% <0.00%> (-0.28%) ⬇️
fury/fury/tests/test_utils.py 94.74% <0.00%> (+0.13%) ⬆️

Copy link
Contributor

@skoudoro skoudoro left a comment

Choose a reason for hiding this comment

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

Hi @frheault,

Thank you for this. We will not add nibabel as a dependency. Unfortunately, you need to change the way you are doing it here.

fury/utils.py Outdated Show resolved Hide resolved
Copy link
Contributor

@skoudoro skoudoro left a comment

Choose a reason for hiding this comment

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

Hi @frheault,

Thank you for the update. Can you look at this last error:

        # Get the 3d points_array
        if not isinstance(lines, list):
>           points_array = lines._data
E           AttributeError: 'numpy.ndarray' object has no attribute '_data'
../fury/utils.py:262: AttributeError
=========================== short test summary info ============================
FAILED ../fury/tests/test_molecular.py::test_bounding_box - AttributeError: '...
========================= 1 failed, 12 passed in 4.03s =========================

Copy link
Contributor

@skoudoro skoudoro left a comment

Choose a reason for hiding this comment

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

Hi @frheault,

Can you do this small change and add a small test with different types of empty streamlines.
After, it should be good to go. I might have merged your other PR a bit fast.

Thank you.

ps: since dipy is optional, we will have to use the decorator skipif and optional package for dipy.

fury/utils.py Outdated Show resolved Hide resolved
Copy link
Contributor

@skoudoro skoudoro left a comment

Choose a reason for hiding this comment

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

Thank you for this @frheault, merging

@skoudoro skoudoro merged commit 07ed101 into fury-gl:master Jun 10, 2022
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.

None yet

2 participants