DOC Rework plot_hashing_vs_dict_vectorizer.py example #23266
Conversation
ogrisel
left a comment
ogrisel
left a comment
There was a problem hiding this comment.
Thanks for the PR, here is a batch of feedback.
Co-authored-by: Olivier Grisel <[email protected]>
Co-authored-by: Olivier Grisel <[email protected]>
ArturoAmorQ
changed the title
ogrisel
left a comment
ogrisel
left a comment
There was a problem hiding this comment.
Thanks very much @ArturoAmorQ, this notebook is much nicer than the original benchmark script.
Here is a final batch of suggestions for improvement:
Co-authored-by: Olivier Grisel <[email protected]>
There was a problem hiding this comment.
Thank you, @ArturoAmorQ.
I think one should use other terms to make this example more accurate.
This is for instance the case of:
- "frequency" which can be replace by "occurence (counts)" (to respect the the definition)
- "speed" which can be replaced by "data processing rate" (to respect the unit (bytes/sec))
Here are some comments and formatting fixes.
Edit: not related to this PR, but #23004 might come with new changes for this example then.
Co-authored-by: Julien Jerphanion <[email protected]>
There was a problem hiding this comment.
Thank you, @ArturoAmorQ.
Edit: I let @ogrisel merge if everything LGTH.
ogrisel
left a comment
ogrisel
left a comment
There was a problem hiding this comment.
LGTM again, just a final batch of nitpicks + a formatting fix.
|
Merged, thank you very much for the nice contribution @ArturoAmorQ! |
…3266) Co-authored-by: Olivier Grisel <[email protected]> Co-authored-by: Julien Jerphanion <[email protected]>
…3266) Co-authored-by: Olivier Grisel <[email protected]> Co-authored-by: Julien Jerphanion <[email protected]>
Co-authored-by: Olivier Grisel <[email protected]> Co-authored-by: Julien Jerphanion <[email protected]>
4 participants
Reference Issues/PRs
Related to #22928
What does this implement/fix? Explain your changes.
In #22928 we remove the use of
HashingVectorizerfrom the plot_document_classification_20newsgroups.py example for the sake of simplicity.A comparison of the performance of hashers and vectorizers can be moved to this existing example.
Any other comments?
Side effect: Implements notebook style as intended in #22406