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

Add TextualSummarization function #159

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hugo-cordoba
Copy link

@hugo-cordoba hugo-cordoba commented Jul 7, 2024

Hi, my name is Hugo.

I am an AI/ML developer based in Spain.

I have opened this PR to contribute to the community. I have developed a function that initializes and returns the summarized text of the videos. Automatically. This is the first time I contribute to this repository, if there is something I did wrong, or something to modify, I'm all ears.

Thanks, see you again! Best regards!!

@hugo-cordoba
Copy link
Author

@microsoft-github-policy-service agree

@shemers
Copy link
Collaborator

shemers commented Jul 10, 2024

Hi @hugo-cordoba,
Thanks for the contribution, we will review it internally and perform code review. it can take some time.


:param video_id: The video ID
:param deploymentName: The name of the deployment
:param length: The length of the summary

Choose a reason for hiding this comment

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

Please specify the allowed values for length and style in the docstring and at the function start - length
The length of the summary. Allowed values: Medium / Short / Long
style- The style of the summary. Allowed values: Neutral / Casual / Formal

Copy link
Author

@hugo-cordoba hugo-cordoba Jul 22, 2024

Choose a reason for hiding this comment

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

Understood, changed it

print(f"Error getting video summary status: {e}")
return
if video_state == 'Processed':
print(f'Here is the textual summary of the video: \n{video_result}')

Choose a reason for hiding this comment

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

The function should return a results in case of success (remember to fix the annotation accordingly, probably -> Optional[dict] will do)

Copy link
Author

@hugo-cordoba hugo-cordoba Jul 22, 2024

Choose a reason for hiding this comment

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

I agree, I modified it as well.

@hugo-cordoba
Copy link
Author

Thanks for the comments, when you can, you can check again that everything is correct now. Best regards @motikadosh !

@hugo-cordoba
Copy link
Author

GM @motikadosh!! Could you please review the changes made? Thank you for your attention to this matter

@hugo-cordoba
Copy link
Author

hi @shemers @motikadosh, can you take a look at the changes and merge this? thanks!

@hugo-cordoba hugo-cordoba changed the title ADD TextualSummarization function Add TextualSummarization function Oct 24, 2024
@shemers
Copy link
Collaborator

shemers commented Oct 27, 2024

@hugo-cordoba Moti will be OOO for a while now, he will approve this once he returns.
thanks.

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.

3 participants