Resolve "Return types for get_frame" - [merged] #20

Closed thompson318 closed 4 years ago

In GitLab by @StephenThompson on Mar 19, 2019, 11:53

Merges 8-frametype -> master

@MattClarkson @ThomasDowrick @GuofangXiao

Can one or more of you have a look at the following. I've made two significant changes to scikit-suregrynditracker 1: I've removed configure, which is now done when you initialise the NDITracker. 2: get frame now returns separate lists for each type of data: tool ID, timestamp, framenumber, tracking data, tracking quality = NDITracker.get_frame()

Does this make it easier to use?

In GitLab by @StephenThompson on Mar 19, 2019, 12:00

Forgot to mention, the tracking data is also returned as a 4x4 NumPy array by default. You can change it to quaternions via the configuration.

In GitLab by @MattClarkson on Mar 20, 2019, 10:43

Code Review Comments:

Looks good. Nice demo.
Code coverage low, presumably due to lack of tracker, so thats fine.
Documentation for start_tracking and stop_tracking :raises not formatted correctly
Unit test, not PEP8, specifically, number of lines between unit tests should be 2.
same problem, NDITracker, not picked up by pylint, but picked up by my PyCharm.

General questions:

Does scikit-surgerytracker need updating? or the Aruco one?
If I build an Atracsys one now, should we build same interface, is it in a base class?

Otherwise:

Looks good, feel free to merge after above formatting issues, and you and @GuofangXiao happy with it.

In GitLab by @StephenThompson on Mar 20, 2019, 12:28

added 4 commits

2f3e8a10 - Issue #8 put testing into lint and tidied up
bfc743f1 - Issue #8 removed tests on protected members
29d73508 - Issue #8 changed raise to raises, to help pycharm
945cc701 - Issue #8 added extra line between unit tests

Compare with previous version

In GitLab by @StephenThompson on Mar 20, 2019, 12:35

Thanks for the comments, have fixed formatting. Except I can't work out what's wrong with NDITracker, as far I can understand it is is PEP8.

I don't think I can do much with coverage, unless we have trackers connected, or implemented fake trackers, which might be fun.

I've parked scikit-surgerytracker, I can't see the point of having a base class in python. But would suggest all data sources follow the same basic interface to make life easier. I have updated scikit-surgeryaruco tracker to have the same interface.

In GitLab by @StephenThompson on Mar 20, 2019, 15:50

mentioned in commit 85bd97f1f197a7cc52553c7fd6dc7aabe1dcd0d6

In GitLab by @StephenThompson on Mar 20, 2019, 15:50

merged

SciKit-Surgery / scikit-surgerynditracker

Resolve "Return types for get_frame" - [merged] #20