Skip to content

Conversation

@Tobias-Fischer
Copy link
Owner

Fix #71

@Tobias-Fischer Tobias-Fischer added the enhancement New feature or request label Jun 22, 2020
@Tobias-Fischer Tobias-Fischer self-assigned this Jun 22, 2020
from tqdm import tqdm

from rt_gene.download_tools import download_external_landmark_models
download_external_landmark_models()
Copy link
Owner Author

Choose a reason for hiding this comment

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

Note that this cannot be moved in the __init__ as the SFDDetector import below requires the models to be existing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

As you quite rightly thought - this is probably the least "nice" bit about this.
We have already modified the SFD detector to add in rospkg - could we not modify it further to make this less "hacky"?

@Tobias-Fischer
Copy link
Owner Author

We could further improve this by only downloading the models once they are required, but I think this adds quite a bit of complexity and would be prone to mistakes.

Copy link
Collaborator

@ahmed-alhindawi ahmed-alhindawi left a comment

Choose a reason for hiding this comment

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

I whole heartidly agree with this general pull commit, but is it worth altering the SFD detector to keep everything neat and tidy?

from tqdm import tqdm

from rt_gene.download_tools import download_external_landmark_models
download_external_landmark_models()
Copy link
Collaborator

Choose a reason for hiding this comment

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

As you quite rightly thought - this is probably the least "nice" bit about this.
We have already modified the SFD detector to add in rospkg - could we not modify it further to make this less "hacky"?

@Tobias-Fischer
Copy link
Owner Author

Alright, what do we think after the latest commit @ahmed-alhindawi? :)

Copy link
Collaborator

@ahmed-alhindawi ahmed-alhindawi left a comment

Choose a reason for hiding this comment

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

w00p.
Looks wonderful, thanks for doing that.

@ahmed-alhindawi ahmed-alhindawi merged commit b185a3b into master Jun 25, 2020
@ahmed-alhindawi ahmed-alhindawi deleted the download_model_refactor branch June 25, 2020 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Download models if not existing

3 participants