Repository navigation
added logger warning and error for cosine similarity computation for … - #203
Merged
Devis Lucato (dluc) merged 11 commits intoApr 4, 2023
Conversation
Alex Chao (alexchaomander)
added
the
python
Contributor
|
This PR addresses #130 |
… for the function
Alex Chao (alexchaomander)
previously approved these changes
Mar 29, 2023
Alex Chao (alexchaomander)
left a comment
Alex Chao (alexchaomander)
left a comment
Contributor
There was a problem hiding this comment.
LGTM!
vivekpandit-msft
dismissed
Alex Chao (alexchaomander)’s stale review
via
April 1, 2023 00:55
f3ff44c
Alex Chao (alexchaomander)
previously approved these changes
Apr 1, 2023
Jordan Henkel (jjhenkel)
suggested changes
Apr 1, 2023
Jordan Henkel (jjhenkel)
left a comment
Jordan Henkel (jjhenkel)
left a comment
There was a problem hiding this comment.
Thanks for this vivekpandit-msft! I left a few requests for fixes. Some other broader requests:
Can you run make pre-commit before and apply fixes, looks like some things are out-of-compliance w/ the linter. This also runs things like isort (which we will be adding to the linting workflow soon).
My output from make pre-commit on this PR:
make pre-commit
poetry run pre-commit run --all-files -c .conf/.pre-commit-config.yaml
Check Toml...............................................................Passed
Check Yaml...............................................................Passed
Fix End of Files.........................................................Passed
Mixed line ending........................................................Passed
black....................................................................Passed
isort....................................................................Failed
- hook id: isort
- files were modified by this hook
Fixing /home/jjhenkel/workspace/semantic-kernel/python/semantic_kernel/core_skills/__init__.py
Fixing /home/jjhenkel/workspace/semantic-kernel/python/semantic_kernel/core_skills/file_io_skill.py
Fixing /home/jjhenkel/workspace/semantic-kernel/python/tests/test_time_skill.py
Fixing /home/jjhenkel/workspace/semantic-kernel/python/semantic_kernel/memory/volatile_memory_store.py
flake8...................................................................Failed
- hook id: flake8
- exit code: 1
python/semantic_kernel/memory/volatile_memory_store.py:57:89: E501 line too long (110 > 88 characters)
python/semantic_kernel/memory/volatile_memory_store.py:58:89: E501 line too long (120 > 88 characters)
python/semantic_kernel/memory/volatile_memory_store.py:65:89: E501 line too long (112 > 88 characters)
python/semantic_kernel/memory/volatile_memory_store.py:89:89: E501 line too long (92 > 88 characters)
Can you structure the tests similar to the others that were recently merged (put them under ./unit/memory/test_volatile_memory_store.py)?
…g pytest asyncio decorator, and resolving code style issues
vivekpandit-msft
dismissed
Alex Chao (alexchaomander)’s stale review
via
April 2, 2023 05:03
f6b467d
Contributor
Author
I have made all the changes you suggested Jordan Henkel (@jjhenkel). Thanks for the review. |
Devis Lucato (dluc)
added
the
PR: feedback to address
Contributor
|
vivekpandit-msft could address the conflict. Jordan Henkel (@jjhenkel) I'll wait for your approval, thanks! |
Jordan Henkel (jjhenkel)
previously approved these changes
Apr 3, 2023
Jordan Henkel (jjhenkel)
left a comment
Jordan Henkel (jjhenkel)
left a comment
There was a problem hiding this comment.
Thanks for the changes vivekpandit-msft! LGTM now, left some minor comments (but mostly things we should just follow-up on, and things that don't apply just to this PR like docstring consistency).
Contributor
Author
Addressed the conflict and Jordan Henkel (@jjhenkel)'s comments |
Devis Lucato (dluc)
approved these changes
Apr 4, 2023
Devis Lucato (dluc)
pushed a commit
that referenced
this pull request
Apr 12, 2023
#203) Added logger warning and error for cosine similarity computation for zero vectors
Devis Lucato (dluc)
pushed a commit
that referenced
this pull request
Apr 13, 2023
#203) Added logger warning and error for cosine similarity computation for zero vectors
Devis Lucato (dluc)
pushed a commit
that referenced
this pull request
Apr 13, 2023
#203) Added logger warning and error for cosine similarity computation for zero vectors
Desmond Howard (dehoward)
pushed a commit
to lemillermicrosoft/semantic-kernel
that referenced
this pull request
Jun 1, 2023
microsoft#203) Added logger warning and error for cosine similarity computation for zero vectors
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added logger warning and error for cosine similarity computation for zero vectors