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

Fix functions_view.py #1213

Merged
merged 9 commits into from
Jun 9, 2023
Merged

Fix functions_view.py #1213

merged 9 commits into from
Jun 9, 2023

Conversation

salmon131
Copy link
Contributor

Motivation and Context

#1212

Description

Fixed a bug in the process of checking native function.

Contribution Checklist

@github-actions github-actions bot added the python Pull requests for the Python Semantic Kernel label May 25, 2023
@alexchaomander
Copy link
Contributor

Thanks for adding this! The team will review soon!

@awharrison-28
Copy link
Contributor

closes #1212

@salmon131 would you mind turning the examples you demonstrated in the above issue into unit tests?

@github-actions github-actions bot added .NET Issue or Pull requests regarding .NET code kernel Issues or pull requests impacting the core kernel kernel.core labels Jun 3, 2023
@github-actions github-actions bot removed .NET Issue or Pull requests regarding .NET code kernel.core kernel Issues or pull requests impacting the core kernel labels Jun 3, 2023
@salmon131
Copy link
Contributor Author

salmon131 commented Jun 3, 2023

@microsoft-github-policy-service agree company="ascentkorea"

@salmon131
Copy link
Contributor Author

salmon131 commented Jun 3, 2023

@awharrison-28
I added a unit test for the example. Existing code fails when performing test_is_native(), but modified code passes the test.

awharrison-28
awharrison-28 previously approved these changes Jun 5, 2023
Copy link
Contributor

@awharrison-28 awharrison-28 left a comment

Choose a reason for hiding this comment

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

Love the tests

@awharrison-28
Copy link
Contributor

to fix the linting issue, run poetry run black . under semantic-kernel/python

I tried to do it myself, but I hit an issue pushing to this PR.

@github-actions github-actions bot added .NET Issue or Pull requests regarding .NET code kernel Issues or pull requests impacting the core kernel samples labels Jun 7, 2023
@github-actions github-actions bot removed the .NET Issue or Pull requests regarding .NET code label Jun 7, 2023
@github-actions github-actions bot removed samples kernel Issues or pull requests impacting the core kernel labels Jun 7, 2023
@salmon131
Copy link
Contributor Author

@awharrison-28
I solved the lint issue. Please check.

@awharrison-28
Copy link
Contributor

@awharrison-28 I solved the lint issue. Please check.

Looks like you'll need to do poetry run ruff check . as well

@salmon131
Copy link
Contributor Author

salmon131 commented Jun 8, 2023

@awharrison-28
I also performed a ruff check. Please check.

@dluc dluc merged commit f60d7ba into microsoft:main Jun 9, 2023
@dluc dluc mentioned this pull request Jun 12, 2023
dluc added a commit that referenced this pull request Jun 12, 2023
* [e4781e5] Add sample notebook to demo weaviate memory store (#1359)
* [dae1c16] Python: Added examples of using ChatCompletion models for
skill building in Jupyter Notebooks (#1242)
* [f4e92eb] fix: Add Azure OpenAI support for
python/08-native-function-inline (#1365)
* [de74668] Fixing typos (#1377)
* [67aa732] Python: Fix weaviate integration tests (#1422)
* [f60d7ba] Fix functions_view.py (#1213)
* [b2e1548] Python: Multiple results per prompt (incl. streaming)
(#1316)
* [4c4670a] Using dotenv instead of parsing keys ourselves (#1295)
* [05d9e72] Python: Sync pyproject.toml with requirements.txt (#1150)
* [6cbea85] Python: Add additional_metadata field to MemoryRecord and
address TODOs in ChromaMemoryStore (#1323)
* [8947e68] Weaviate: Fix to be compatible with python 3.8 (#1349)
shawncal pushed a commit to shawncal/semantic-kernel that referenced this pull request Jul 6, 2023
### Motivation and Context
microsoft#1212 

### Description
Fixed a bug in the process of checking native function.


---------

Co-authored-by: Abby Harrison <54643756+awharrison-28@users.noreply.github.com>
Co-authored-by: Devis Lucato <dluc@users.noreply.github.com>
shawncal pushed a commit to shawncal/semantic-kernel that referenced this pull request Jul 6, 2023
* [e4781e5] Add sample notebook to demo weaviate memory store (microsoft#1359)
* [dae1c16] Python: Added examples of using ChatCompletion models for
skill building in Jupyter Notebooks (microsoft#1242)
* [f4e92eb] fix: Add Azure OpenAI support for
python/08-native-function-inline (microsoft#1365)
* [de74668] Fixing typos (microsoft#1377)
* [67aa732] Python: Fix weaviate integration tests (microsoft#1422)
* [f60d7ba] Fix functions_view.py (microsoft#1213)
* [b2e1548] Python: Multiple results per prompt (incl. streaming)
(microsoft#1316)
* [4c4670a] Using dotenv instead of parsing keys ourselves (microsoft#1295)
* [05d9e72] Python: Sync pyproject.toml with requirements.txt (microsoft#1150)
* [6cbea85] Python: Add additional_metadata field to MemoryRecord and
address TODOs in ChromaMemoryStore (microsoft#1323)
* [8947e68] Weaviate: Fix to be compatible with python 3.8 (microsoft#1349)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Pull requests for the Python Semantic Kernel
Projects
No open projects
Status: Released
Development

Successfully merging this pull request may close these issues.

4 participants