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

Import video_calibration and video_calibration_checker apps from scikit-surgerycalibration.py #46

Open
MattClarkson opened this issue Mar 18, 2022 · 2 comments
Labels
p2-medium Medium priority

Comments

@MattClarkson
Copy link
Collaborator

No description provided.

MattClarkson added a commit that referenced this issue May 30, 2022
MattClarkson added a commit that referenced this issue May 30, 2022
MattClarkson added a commit that referenced this issue May 30, 2022
MattClarkson added a commit that referenced this issue May 30, 2022
@MattClarkson
Copy link
Collaborator Author

@thompson318 , @tdowrick

Would anyone like to review code on branch 46-use-headless?

In order to use opencv-headless, I had to import videocalibration_app and videocalibrationchecker_app from scikit-surgerycalibration, and then re-write the UI to use PySide (VTKOverlayWindow).

So, its not worth doing a diff with master ... as the 4 main files are a complete re-write.

so, please can someone just look at:

  • sksurgeryutils/ui/sksurgeryvideocalibration_command_line.py
  • sksurgeryutils/ui/sksurgeryvideocalibration_app.py
  • sksurgeryutils/ui/sksurgeryvideocalibrationchecker_command_line.py
  • sksurgeryutils/ui/sksurgeryvideocalibrationchecker_app.py

I also made the decision that things that are static config should be in the config file, and things that you might change on the fly, and hence would not want to git commit, are command line args, e.g. opencv source, interactive mode etc.

Overall, I'm happy with the result, unit tests pass, linting passes, but of course, happy for suggestions.

@thompson318
Copy link
Collaborator

Looks good to me thank you. It only seems to be passing on python 3.7 at the moment. Is that intentional?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2-medium Medium priority
Projects
None yet
Development

No branches or pull requests

3 participants