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

Exercises for the AutoSxS LLM evaluation lab #445

Merged
merged 2 commits into from
Apr 18, 2024
Merged

Conversation

BenoitDherin
Copy link
Collaborator

No description provided.

@BenoitDherin BenoitDherin self-assigned this Apr 17, 2024
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@BenoitDherin BenoitDherin requested a review from takumiohym April 17, 2024 22:28
@@ -0,0 +1,653 @@
{
Copy link
Collaborator

@takumiohym takumiohym Apr 18, 2024

Choose a reason for hiding this comment

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

Line #5.    parameters = None  # TODO

Maybe just a matter of preference, but I prefer using three dots operator ... , like parameters = ... which explicitly tells this code is not complete, instead of None .


Reply via ReviewNB

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the "..." triggers issues with pre-commit, while "None" is okay because regular Python syntax.

Copy link
Collaborator

Choose a reason for hiding this comment

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

... is a regular Python expression, so ok for pre-commit as well.

@@ -0,0 +1,653 @@
{
Copy link
Collaborator

@takumiohym takumiohym Apr 18, 2024

Choose a reason for hiding this comment

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

Line #3.            break

It seems the old code is still remaining.


Reply via ReviewNB

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch! Done!

@BenoitDherin BenoitDherin merged commit 0d8f3f2 into master Apr 18, 2024
5 checks passed
@BenoitDherin BenoitDherin deleted the autosxs-exercises branch April 18, 2024 20:27
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.

2 participants