Fix the sdf bug by checking the arguments passed

[tushar5526]

Closes #310
If enough primitives are not provided, we default to a torus. Another possible solution could be trying to broadcast the NumPy arrays before defaulting to a torus.

[pep8speaks]

Hello @tushar5526! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-01-07 20:26:40 UTC

[skoudoro]

Hi @tushar5526,

Thank you for this. Overall, it looks good but could you add a test, please?

Also, I need to test it locally.

Thank you

[tushar5526]

Sure @skoudoro. Thanks for the review 👍

[tushar5526]

I have added the tests @skoudoro.
test_sdf_actor.py is getting passed, however,

FAILED tests/test_actors.py::test_grid - AssertionError: 
FAILED tests/test_actors.py::test_matplotlib_figure - AssertionError: 

are failing by default.

[Nibba2018]

Hi @tushar5526 , A quick tip, you can do flake8 path/to/file to check PEP8 issues. Also can you tell what failures are you having for those 2 tests? You can do pytest -svv path/to/file/test -k <name_of_test_method> to test a specific test.

[tushar5526]

Hi @tushar5526 , A quick tip, you can do flake8 path/to/file to check PEP8 issues

Thank you for the tip @Nibba2018. Finally no PEP issues :p

Also can you tell what failures are you having for those 2 tests?

These two tests are failing on the latest master-branch too. Here is the full summary

__________________________________ test_grid ___________________________________

_interactive = False

    def test_grid(_interactive=False):
    
        vol1 = np.zeros((100, 100, 100))
        vol1[25:75, 25:75, 25:75] = 100
        contour_actor1 = actor.contour_from_roi(vol1, np.eye(4),
                                                (1., 0, 0), 1.)
    
        vol2 = np.zeros((100, 100, 100))
        vol2[25:75, 25:75, 25:75] = 100
    
        contour_actor2 = actor.contour_from_roi(vol2, np.eye(4),
                                                (1., 0.5, 0), 1.)
        vol3 = np.zeros((100, 100, 100))
        vol3[25:75, 25:75, 25:75] = 100
    
        contour_actor3 = actor.contour_from_roi(vol3, np.eye(4),
                                                (1., 0.5, 0.5), 1.)
    
        scene = window.Scene()
        actors = []
        texts = []
    
        actors.append(contour_actor1)
        text_actor1 = actor.text_3d('cube 1', justification='center')
        texts.append(text_actor1)
    
        actors.append(contour_actor2)
        text_actor2 = actor.text_3d('cube 2', justification='center')
        texts.append(text_actor2)
    
        actors.append(contour_actor3)
        text_actor3 = actor.text_3d('cube 3', justification='center')
        texts.append(text_actor3)
    
        actors.append(shallow_copy(contour_actor1))
        text_actor1 = 'cube 4'
        texts.append(text_actor1)
    
        actors.append(shallow_copy(contour_actor2))
        text_actor2 = 'cube 5'
        texts.append(text_actor2)
    
        actors.append(shallow_copy(contour_actor3))
        text_actor3 = 'cube 6'
        texts.append(text_actor3)
    
        # show the grid without the captions
        container = grid(actors=actors, captions=None,
                         caption_offset=(0, -40, 0),
                         cell_padding=(10, 10), dim=(2, 3))
    
        scene.add(container)
    
        scene.projection('orthogonal')
    
        counter = itertools.count()
    
        show_m = window.ShowManager(scene)
    
        show_m.initialize()
    
        def timer_callback(_obj, _event):
            cnt = next(counter)
            # show_m.scene.zoom(1)
            show_m.render()
            if cnt == 4:
                show_m.exit()
                show_m.destroy_timers()
    
        show_m.add_timer_callback(True, 200, timer_callback)
        show_m.start()
    
        arr = window.snapshot(scene)
        report = window.analyze_snapshot(arr)
>       npt.assert_equal(report.objects, 6)
E       AssertionError: 
E       Items are not equal:
E        ACTUAL: 10
E        DESIRED: 6

tests/test_actors.py:1154: AssertionError
____________________________ test_matplotlib_figure ____________________________

    @pytest.mark.skipif(not have_matplotlib, reason="Requires MatplotLib")
    def test_matplotlib_figure():
    
        names = ['group_a', 'group_b', 'group_c']
        values = [1, 10, 100]
    
        fig = plt.figure(figsize=(9, 3))
    
        plt.subplot(131)
        plt.bar(names, values)
        plt.subplot(132)
        plt.scatter(names, values)
        plt.subplot(133)
        plt.plot(names, values)
        plt.suptitle('Categorical Plotting')
    
        arr = matplotlib_figure_to_numpy(fig, dpi=500, transparent=True)
        fig_actor = actor.figure(arr, 'cubic')
        fig_actor2 = actor.figure(arr, 'cubic')
        scene = window.Scene()
        scene.background((1, 1, 1.))
    
        ax_actor = actor.axes(scale=(1000, 1000, 1000))
        scene.add(ax_actor)
        scene.add(fig_actor)
        scene.add(fig_actor2)
        ax_actor.SetPosition(0, 500, -800)
        fig_actor2.SetPosition(500, 800, -400)
        display = window.snapshot(scene, 'test_mpl.png', order_transparent=False,
                                  offscreen=True)
        res = window.analyze_snapshot(display, bg_color=(255, 255, 255.),
                                      colors=[(31, 119, 180), (255, 0, 0)],
                                      find_objects=False)
>       npt.assert_equal(res.colors_found, [True, True])
E       AssertionError: 
E       Items are not equal:
E       item=1
E       
E        ACTUAL: False
E        DESIRED: True

tests/test_actors.py:1268: AssertionError

[Nibba2018]

@tushar5526 Your branch is failing on the same tests on my environment too.

[tushar5526]

@Nibba2018 Yes, those tests are failing due to some other issue, and not due to any changes made in this PR.

[skoudoro]

Yes, we need to make those tests more robust.

But your code looks good. Thank you for this fix @tushar5526! merging