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.

Sign up for GitHub

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

Jump to bottom

Asaflevi/feature/healthinsights python sdk #29339

Merged
merged 22 commits into from
Mar 30, 2023
Merged

Asaflevi/feature/healthinsights python sdk #29339

merged 22 commits into from
Mar 30, 2023

Conversation

asaflevi-ms
Copy link
Member

Description

first PR for Health Insights first public preview.
Cadl PR: Azure/azure-rest-api-specs#22990

Please add an informative description that covers that changes made by the pull request and link all relevant issues.

If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

@asaflevi-ms asaflevi-ms requested a review from lmazuel as a code owner March 14, 2023 17:13
@asaflevi-ms
Copy link
Member Author

Hi @lmazuel .
This is the Python SDK PR for the Health Insights first public preview.
Cadl PR: Azure/azure-rest-api-specs#22990

@asaflevi-ms
Copy link
Member Author

/azp run prepare-pipelines

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@weshaggard
Copy link
Member

/azp run python - healthinsights - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@azure-sdk
Copy link
Collaborator

azure-sdk commented Mar 15, 2023 •
edited
Loading

API change check

APIView has identified API level changes in this PR and created following API reviews.

azure-healthinsights-cancerprofiling
azure-healthinsights-clinicalmatching

@msyyc msyyc mentioned this pull request Mar 20, 2023
Closed
@lirenhe
Copy link
Member

lirenhe commented Mar 22, 2023

cc @Azure/dpg-devs for awareness

@msyyc
Copy link
Member

msyyc commented Mar 22, 2023

Hi @asaflevi-ms Did you generate SDK code with swagger and autorest.python or typespec and typespec-python?

@asaflevi-ms
Copy link
Member Author

Hi @asaflevi-ms Did you generate SDK code with swagger and autorest.python or typespec and typespec-python?

typespec
Azure/azure-rest-api-specs#22990

@asaflevi-ms
Copy link
Member Author

Hii, @msyyc
We have public preview next week.
Please let me know what else can I do to get approved and merged.

@msyyc
Copy link
Member

msyyc commented Mar 27, 2023

@asaflevi-ms
(1) The PR overall looks fine. Please update changelog date with guidance: https://github.com/Azure/azure-sdk-for-python/wiki/What-to-do-after-generating-the-SDK-code-with-codegen#changelogmd
(2) Did you pass arcboard review about namespace for this two new package? If not, please follow https://dev.azure.com/azure-sdk/internal/_wiki/wikis/internal.wiki/821/Naming-for-new-initial-management-or-client-libraries-(new-SDKs)

@asaflevi-ms
Copy link
Member Author

@asaflevi-ms (1) The PR overall looks fine. Please update changelog date with guidance: https://github.com/Azure/azure-sdk-for-python/wiki/What-to-do-after-generating-the-SDK-code-with-codegen#changelogmd (2) Did you pass arcboard review about namespace for this two new package? If not, please follow https://dev.azure.com/azure-sdk/internal/_wiki/wikis/internal.wiki/821/Naming-for-new-initial-management-or-client-libraries-(new-SDKs)

Thanks @msyyc
Yes we had archboard review and names/namespaces approved.

@msyyc
Copy link
Member

msyyc commented Mar 28, 2023

@asaflevi-ms I guess you transfer the sample code and readme from .net. Python is very different with .net so I advise you run the sample code locally to make sure they work as expected.

@msyyc msyyc force-pushed the asaflevi/feature/healthinsights-python-sdk branch from 70de6d4 to 64d5347 Compare March 29, 2023 08:27
@msyyc
Copy link
Member

msyyc commented Mar 29, 2023

add @iscai-msft to review.

@msyyc msyyc requested a review from iscai-msft March 29, 2023 09:19
iscai-msft
iscai-msft reviewed Mar 29, 2023
View reviewed changes
Copy link
Contributor

@iscai-msft iscai-msft left a comment

Choose a reason for hiding this comment

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

can you split azure-healthinsights-clinicalmatching into a different PR?

sdk/healthinsights/azure-healthinsights-cancerprofiling/README.md Outdated Show resolved Hide resolved
sdk/healthinsights/azure-healthinsights-cancerprofiling/README.md Outdated Show resolved Hide resolved
sdk/healthinsights/azure-healthinsights-cancerprofiling/README.md Outdated Show resolved Hide resolved
sdk/healthinsights/azure-healthinsights-cancerprofiling/README.md Outdated Show resolved Hide resolved
sdk/healthinsights/azure-healthinsights-cancerprofiling/README.md Show resolved Hide resolved
...healthinsights/azure-healthinsights-cancerprofiling/samples/sample_infer_cancer_profiling.py Outdated Show resolved Hide resolved
@msyyc
Copy link
Member

msyyc commented Mar 30, 2023

merge it since @asaflevi-ms will start another PR to update doc links of README.md.

@msyyc msyyc merged commit c5954d4 into Azure:main Mar 30, 2023
@asaflevi-ms asaflevi-ms deleted the asaflevi/feature/healthinsights-python-sdk branch March 30, 2023 12:29
@asaflevi-ms asaflevi-ms mentioned this pull request Mar 30, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iscai-msft iscai-msft iscai-msft left review comments

@xiangyan99 xiangyan99 xiangyan99 left review comments

@msyyc msyyc msyyc approved these changes

@lmazuel lmazuel Awaiting requested review from lmazuel

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@asaflevi-ms @weshaggard @azure-sdk @lirenhe @msyyc @lmazuel @xiangyan99 @iscai-msft