Fixes some bugs from ShowManager Object.

[devmessias]

Related with #483 This PR adress two minor issues with the ShowManager object

1. destroy timer behavior

ShowManager destroy_timer method destroys the timer without removing the callback related with the observer. Thus, we lost the reference for the callback. Because of that, another timer event created after that will keep calling the callback function with the missed reference.

2. Size Window attribute

This is more trickly and usually happens on Windows. The vtk creates a window instance with a slightly different size. Therefore, the following test can fail:

size = (300, 400)
showm = ShowManager(size=size)
# code goes here
assert size[0] = showm.size[0] and size[1] == showm.size[1]
I noticed that bug running the tests of PR #437 on windows github actions.

This is an example which shows the bug solved by this PR.

from fury.window import ShowManager
from fury.actor import sphere
import numpy as np

centers = np.array([[1, 0,0]])
showm = ShowManager(size=(400, 400))
i1 = 0
i2 = 0

#check_bug = False  

def t1(_, __):
    global i1
    i1 += 1
    print(f'timer 1 event {i1}')

def t2(_, __):
    global i2
    i2 += 1
    print(f'timer 2 event {i2}')

id1 = showm.add_timer_callback(False, 1, t1)
print(id1, showm._timers)
id2 = showm.add_timer_callback(False, 2, t2)

print(id2, showm._timers)
showm.destroy_timer(id1)

showm.initialize()

color = np.array([0, 1, 0])
actor = sphere(centers, color)
showm.scene.add(actor)

showm.start()
#showm.render()
assert i1 == 0 # the code will fail here

After this PR, the above code should work whithout problems

[codecov[bot]]

Codecov Report

Merging #484 (740078f) into master (2a1b5a3) will decrease coverage by 0.04%. The diff coverage is 72.22%.

:exclamation: Current head 740078f differs from pull request most recent head 41ade7a. Consider uploading reports for the commit 41ade7a to get more accurate results

@@            Coverage Diff             @@
##           master     #484      +/-   ##
==========================================
- Coverage   88.60%   88.56%   -0.05%     
==========================================
  Files          31       31              
  Lines        6617     6583      -34     
  Branches      794      787       -7     
==========================================
- Hits         5863     5830      -33     
- Misses        535      536       +1     
+ Partials      219      217       -2     

Impacted Files	Coverage Δ
fury/window.py	81.10% <72.22%> (+0.94%)	:arrow_up:
fury/io.py	78.57% <0.00%> (-2.68%)	:arrow_down:
fury/ui/tests/test_containers.py	91.44% <0.00%> (-0.66%)	:arrow_down:
fury/layout.py	100.00% <0.00%> (ø)

[devmessias]

Hi @devmessias,

Looks good to me. Can you add some tests to make sure that the detected bug will not appear again?

After that, we should merge this PR. Thanks!

Hi @skoudoro. I'm trying to figure out how to test this whithout using showm.start(). I couldn't find a method inside of vtkRenderWindowInteractor which returns the list of observers and timers. In a vtkActor instance we have GetMapper()

[pep8speaks]

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

• In the file fury/tests/test_window.py:

Line 373:80: E501 line too long (89 > 79 characters) Line 379:46: E231 missing whitespace after ':'

Comment last updated at 2021-09-30 13:57:58 UTC

[Garyfallidis]

@devmessias this PR needs more work. First of all, you need to explain in the description which exact bugs you are fixing. Referencing another issue is not enough. Also you have to explain a bit on the test what you are testing. Currently it is not clear. At least in Windows I do not see the expected behavior and when I enable interactive = True I get this error:

Traceback (most recent call last):
  File "test_window.py", line 490, in <module>
    test_timers()
  File "test_window.py", line 391, in test_timers
    npt.assert_equal(counts_by_interval[1] >= ms_3//ms_2, True)
  File "C:\Users\elef\AppData\Local\Continuum\anaconda3\lib\site-packages\numpy\testing\_private\utils.py", line 427, in assert_equal
    raise AssertionError(msg)
AssertionError:
Items are not equal:
 ACTUAL: False
 DESIRED: True

Also there are still some free flying comments and pep 8 issues. Please address the issues reported here and fix the title too. The title is very generic and does not help reviewing. Thanks in advance!

[Garyfallidis]

Ping @devmessias!

[skoudoro]

Hi @devmessias,

Thank you for your update! Could you also rebase to resolve those conflicts?