-
Notifications
You must be signed in to change notification settings - Fork 908
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
Change nvtext::load_vocabulary_file to return a unique ptr #7424
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a code smell. There is rarely a reason to return a shared_ptr
from a function. It should likely return a unique_ptr
and if someone desires a shared_ptr
they can trivially make it into one.
Just commenting that from the Python side we can handle a |
Codecov Report
@@ Coverage Diff @@
## branch-0.19 #7424 +/- ##
==============================================
Coverage ? 82.21%
==============================================
Files ? 101
Lines ? 17057
Branches ? 0
==============================================
Hits ? 14023
Misses ? 3034
Partials ? 0 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@gpucibot merge |
Reference #5868
This PR changes the
nvtext::load_vocabulary_file
to return a unique-pointer to make it easier to manage in Python/Cython class object. The original signature returned a flat structure that contained unique-pointers which would make it difficult to copy and manage.The corresponding gtests and gbenchmarks were updated for this API change.