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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

.Net: Obsolete SKContext[...] indexer #2188

Merged
merged 4 commits into from
Jul 26, 2023

Conversation

shawncal
Copy link
Member

Motivation and Context

The SKContext[...] indexer is simply shorthand for SKContext.Variables[...]. With additional routes to doing the same thing, there's more confusion about which to use when. In this case, there's a great deal of confusion about the mutability of these variables and whether they can/should be changed within functions or between functions.

Description

This PR starts to address the above issues by simplifying the SDK surface. The first step is to remove the "extra" route to accessing the same data, the indexer on SKContext.

This is a breaking change. The mitigation is simply to update all uses of context["MY_VALUE"] to context.Variables["MY_VALUE"]

In the PRs that follow, the mutability issue will be addressed by making it clear which variables are input, how and where to set output, and which can/cannot be transformed by an SKFunction.

Contribution Checklist

@shawncal shawncal added .NET Issue or Pull requests regarding .NET code kernel.core PR: breaking change Pull requests that introduce breaking changes labels Jul 26, 2023
@shawncal shawncal requested a review from a team as a code owner July 26, 2023 00:23
@shawncal shawncal added the kernel Issues or pull requests impacting the core kernel label Jul 26, 2023
stephentoub
stephentoub previously approved these changes Jul 26, 2023
RogerBarreto
RogerBarreto previously approved these changes Jul 26, 2023
RogerBarreto
RogerBarreto previously approved these changes Jul 26, 2023
@shawncal shawncal added this pull request to the merge queue Jul 26, 2023
Merged via the queue into microsoft:main with commit 32447c0 Jul 26, 2023
13 checks passed
@shawncal shawncal deleted the skcontext-indexer-removal branch July 26, 2023 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kernel Issues or pull requests impacting the core kernel .NET Issue or Pull requests regarding .NET code PR: breaking change Pull requests that introduce breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants