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

docs: extend tutorial14 about query classification #3013

Merged
merged 23 commits into from Aug 12, 2022
Merged

docs: extend tutorial14 about query classification #3013

merged 23 commits into from Aug 12, 2022

Conversation

anakin87
Copy link
Member

@anakin87 anakin87 commented Aug 9, 2022

As stated in #2965, it would be good to include in Tutorial 14 a small section about new features of TransformersQueryClassifier:

  • using custom transformer models
  • using zero-shot-classification

Proposed Changes:

I added a small section on these features, similar to what has been done for the features already present.
I'm not that good with documentation, so I'd beg the people involved (@agnieszka-m @TuanaCelik @mkkuemmel) to brutally make the necessary changes 😃

Checklist

@anakin87 anakin87 requested review from a team as code owners August 9, 2022 16:01
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@TuanaCelik
Copy link
Member

@agnieszka-m, looping you in here as I think you'd like to have a look at the language. @anakin87 I'll also put in a review 🙂

@TuanaCelik
Copy link
Member

Overall I think this is great. I would have one suggestion though, as it's a tutorial, I think it might be useful to be a bit more explicit about the difference of zero-shot vs custom classification. The reader might not understand the difference in the current version. For example it starts off with how for custom, you can use a model from the hub. But then you see that you can also use one for zero-shot. The other thing is they might not understand the difference of you defining your own labels like 'cinema' and 'music' being any different to the other labels which are simply 'label1' etc.

All in all some more hand holding go well for a tutorial 👍🏽

Thank you for the contribution!! 🎉

@TuanaCelik TuanaCelik self-requested a review August 9, 2022 23:04
@agnieszka-m
Copy link
Contributor

Hi! Since my changes are mostly minor language corrections, would you like me to do them directly on your files or would you rather I add comments to the files? Whatever works best for you.

@anakin87
Copy link
Member Author

@TuanaCelik I made some improvements. Please let me know WDYT... 😃

Hi! Since my changes are mostly minor language corrections, would you like me to do them directly on your files or would you rather I add comments to the files? Whatever works best for you.

@agnieszka-m for me it is the same...

Copy link
Contributor

@agnieszka-m agnieszka-m left a comment

Choose a reason for hiding this comment

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

Added my two cents.

tutorials/Tutorial14_Query_Classifier.ipynb Outdated Show resolved Hide resolved
tutorials/Tutorial14_Query_Classifier.ipynb Outdated Show resolved Hide resolved
tutorials/Tutorial14_Query_Classifier.ipynb Outdated Show resolved Hide resolved
tutorials/Tutorial14_Query_Classifier.ipynb Outdated Show resolved Hide resolved
tutorials/Tutorial14_Query_Classifier.ipynb Outdated Show resolved Hide resolved
tutorials/Tutorial14_Query_Classifier.ipynb Outdated Show resolved Hide resolved
tutorials/Tutorial14_Query_Classifier.ipynb Outdated Show resolved Hide resolved
tutorials/Tutorial14_Query_Classifier.ipynb Outdated Show resolved Hide resolved
tutorials/Tutorial14_Query_Classifier.ipynb Outdated Show resolved Hide resolved
tutorials/Tutorial14_Query_Classifier.ipynb Outdated Show resolved Hide resolved
anakin87 and others added 3 commits August 10, 2022 11:43
Co-authored-by: Agnieszka Marzec <97166305+agnieszka-m@users.noreply.github.com>
@anakin87
Copy link
Member Author

When the notebook tutorial is acceptable, I will add the .py version as well.

Copy link
Member

@tstadel tstadel left a comment

Choose a reason for hiding this comment

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

I like it very much, nice examples.
Btw, we're planning to make zero-shot query classification one of the highlights of our next release (probably monday). So it would be awesome to have this PR.

Copy link
Member

@TuanaCelik TuanaCelik left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks @anakin87

docs/_src/tutorials/tutorials/14.md Outdated Show resolved Hide resolved
@anakin87
Copy link
Member Author

I've made some minor fixes and added the .py tutorial.
In the next few days, I won't be online ⛱️, so feel free to make corrections (if needed) and take charge of this PR.

@anakin87 anakin87 marked this pull request as draft August 11, 2022 07:17
@anakin87 anakin87 marked this pull request as ready for review August 11, 2022 07:18
Copy link
Contributor

@agnieszka-m agnieszka-m left a comment

Choose a reason for hiding this comment

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

Minor lg corrections

tutorials/Tutorial14_Query_Classifier.ipynb Outdated Show resolved Hide resolved
tutorials/Tutorial14_Query_Classifier.ipynb Outdated Show resolved Hide resolved
tutorials/Tutorial14_Query_Classifier.ipynb Outdated Show resolved Hide resolved
tutorials/Tutorial14_Query_Classifier.ipynb Outdated Show resolved Hide resolved
anakin87 and others added 4 commits August 11, 2022 18:45
Co-authored-by: Agnieszka Marzec <97166305+agnieszka-m@users.noreply.github.com>
Co-authored-by: Agnieszka Marzec <97166305+agnieszka-m@users.noreply.github.com>
Co-authored-by: Agnieszka Marzec <97166305+agnieszka-m@users.noreply.github.com>
Co-authored-by: Agnieszka Marzec <97166305+agnieszka-m@users.noreply.github.com>
@tstadel
Copy link
Member

tstadel commented Aug 11, 2022

@anakin87 this is almost good to go. Only the tutorial script is missing. I'm pushing it over the finish line.

@tstadel
Copy link
Member

tstadel commented Aug 11, 2022

@anakin87 unfortunately I don't have write access. Can you run the script yourself or grant me access?

@anakin87
Copy link
Member Author

'Allow edits and access to secrets by maintainers' is already selected. What can I do to allow you?

@tstadel
Copy link
Member

tstadel commented Aug 12, 2022

'Allow edits and access to secrets by maintainers' is already selected. What can I do to allow you?

@anakin87 Ok, weird. To fix this quickly, can you grant me access individually maybe?

@anakin87
Copy link
Member Author

I invited you to collaborate on my fork... Hope it helps...

@tstadel
Copy link
Member

tstadel commented Aug 12, 2022

@anakin87 Yes thanks! Now it's working.

@tstadel tstadel merged commit 4f261a4 into deepset-ai:master Aug 12, 2022
@anakin87 anakin87 deleted the extend_tutorial14 branch August 16, 2022 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants