Skip to content

updates for Microsoft.DotNet.Interactive.Documents API stabilization #3169

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

Merged
merged 1 commit into from
Sep 11, 2023
Merged

updates for Microsoft.DotNet.Interactive.Documents API stabilization #3169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 3 additions & 10 deletions ...ive.ApiCompatibility.Tests/ApiCompatibilityTests.Document_api_is_not_changed.approved.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ Microsoft.DotNet.Interactive.Documents
public System.Collections.Generic.IAsyncEnumerable<InteractiveDocument> GetImportsAsync(System.Boolean recursive = False)
public System.Collections.Generic.IEnumerable<InputField> GetInputFields()
public System.Collections.Generic.IEnumerable<System.String> GetMagicCommandLines()
public System.Boolean TryGetKernelInfosFromMetadata(ref KernelInfoCollection& kernelInfos)
public class InteractiveDocumentElement
.ctor()
.ctor(System.String contents = null, System.String kernelName = null, System.Collections.Generic.IEnumerable<InteractiveDocumentOutputElement> outputs = null)
Expand Down Expand Up @@ -60,30 +61,22 @@ Microsoft.DotNet.Interactive.Documents
public System.Boolean Contains(System.String nameOrAlias)
public System.Void CopyTo(KernelInfo[] array, System.Int32 arrayIndex)
public System.Boolean Remove(KernelInfo item)
public System.Boolean TryGetByAlias(System.String alias, ref KernelInfo& info)
public class ReturnValueElement : InteractiveDocumentOutputElement, IDataElement
.ctor(System.Collections.Generic.IDictionary<System.String,System.Object> data)
.ctor()
public System.Collections.Generic.IDictionary<System.String,System.Object> Data { get;}
public System.Int32 ExecutionOrder { get; set;}
public System.Collections.Generic.IDictionary<System.String,System.Object> Metadata { get;}
public class TextElement : InteractiveDocumentOutputElement
.ctor(System.String text, System.String name)
.ctor(System.String text, System.String name = stdout)
public System.String Name { get;}
public System.String Text { get;}
Microsoft.DotNet.Interactive.Documents.Jupyter
public class InputCellMetadata
.ctor(System.String kernelName = null, System.String language = null)
public System.String KernelName { get;}
public System.String Language { get;}
public static class InteractiveDocumentExtensions
public static Microsoft.DotNet.Interactive.Documents.InteractiveDocument WithJupyterMetadata(System.String language = C#)
public static class Notebook
public static System.Text.Encoding Encoding { get;}
public static System.Text.Json.JsonSerializerOptions JsonSerializerOptions { get;}
public static Microsoft.DotNet.Interactive.Documents.InteractiveDocument Parse(System.String json, Microsoft.DotNet.Interactive.Documents.KernelInfoCollection kernelInfos = null)
public static Microsoft.DotNet.Interactive.Documents.InteractiveDocument Read(System.IO.Stream stream, Microsoft.DotNet.Interactive.Documents.KernelInfoCollection kernelInfos)
public static System.String ToJupyterJson(System.String defaultLanguage = null)
public static System.Void Write(Microsoft.DotNet.Interactive.Documents.InteractiveDocument document, System.IO.Stream stream)
public static System.Void Write(Microsoft.DotNet.Interactive.Documents.InteractiveDocument document, System.IO.Stream stream, Microsoft.DotNet.Interactive.Documents.KernelInfoCollection kernelInfos)
public static System.Void Write(Microsoft.DotNet.Interactive.Documents.InteractiveDocument document, System.IO.TextWriter writer)
public static System.Void Write(Microsoft.DotNet.Interactive.Documents.InteractiveDocument document, System.IO.TextWriter writer, Microsoft.DotNet.Interactive.Documents.KernelInfoCollection kernelInfos)
4 changes: 2 additions & 2 deletions src/Microsoft.DotNet.Interactive.Documents.Tests/JupyterFormatTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public void notebook_metadata_default_language_is_honored_in_cells_without_langu
[InlineData("PowerShell", "powershell")]
public void Metadata_default_kernel_name_is_based_on_specified_language(string languageName, string kernelName)
{
var document = new InteractiveDocument().WithJupyterMetadata(languageName);
var document = Notebook.Parse(new InteractiveDocument().ToJupyterJson(languageName));

document.GetDefaultKernelName()
.Should()
Expand All @@ -102,7 +102,7 @@ public void Metadata_default_kernel_name_is_based_on_specified_language(string l
[InlineData("PowerShell", "powershell")]
public void Metadata_default_kernel_name_is_based_on_specified_language_when_serialized_and_deserialized(string languageName, string kernelName)
{
var originalDoc = new InteractiveDocument().WithJupyterMetadata(languageName);
var originalDoc = Notebook.Parse(new InteractiveDocument().ToJupyterJson(languageName));

var parsedDoc = Notebook.Parse(originalDoc.ToJupyterJson());

Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.DotNet.Interactive.Documents/CodeSubmission.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public static InteractiveDocument Parse(

metadata = JsonSerializer.Deserialize<Dictionary<string, object>>(metadataString, InteractiveDocument.JsonSerializerOptions);

if (InteractiveDocument.TryGetKernelInfoFromMetadata(metadata, out var kernelInfoFromMetadata))
if (InteractiveDocument.TryGetKernelInfosFromMetadata(metadata, out var kernelInfoFromMetadata))
{
InteractiveDocument.MergeKernelInfos(kernelInfos, kernelInfoFromMetadata);
document.Metadata["kernelInfo"] = kernelInfoFromMetadata;
Expand Down
39 changes: 19 additions & 20 deletions src/Microsoft.DotNet.Interactive.Documents/InteractiveDocument.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public async IAsyncEnumerable<InteractiveDocument> GetImportsAsync(bool recursiv
{
EnsureImportFieldParserIsInitialized();

if (!TryGetKernelInfoFromMetadata(Metadata, out var kernelInfos))
if (!TryGetKernelInfosFromMetadata(out var kernelInfos))
Copy link
Contributor

Choose a reason for hiding this comment

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

minor nit: This call uses the public instance overload while others below use the static overload. Could we standardize to the same call?

{
kernelInfos = new();
}
Expand Down Expand Up @@ -198,7 +198,7 @@ public static async Task<InteractiveDocument> LoadAsync(

public string? GetDefaultKernelName()
{
if (TryGetKernelInfoFromMetadata(Metadata, out var kernelInfo))
if (TryGetKernelInfosFromMetadata(Metadata, out var kernelInfo))
{
return kernelInfo.DefaultKernelName;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the public API missing the remaining heuristics (like checking for kernelspec metadata)

Expand All @@ -208,16 +208,11 @@ public static async Task<InteractiveDocument> LoadAsync(

internal string? GetDefaultKernelName(KernelInfoCollection kernelInfos)
{
if (TryGetKernelInfoFromMetadata(Metadata, out var kernelInfoCollection))
if (TryGetKernelInfosFromMetadata(Metadata, out var kernelInfoCollection))
{
return kernelInfoCollection.DefaultKernelName;
}

if (Metadata is null)
{
return null;
}

if (Metadata.TryGetValue("kernelspec", out var kernelspecObj))
{
if (kernelspecObj is IDictionary<string, object> kernelspecDict)
Expand All @@ -243,7 +238,7 @@ public static async Task<InteractiveDocument> LoadAsync(

internal static void MergeKernelInfos(InteractiveDocument document, KernelInfoCollection kernelInfos)
{
if (TryGetKernelInfoFromMetadata(document.Metadata, out var kernelInfoCollection))
if (TryGetKernelInfosFromMetadata(document.Metadata, out var kernelInfoCollection))
{
MergeKernelInfos(kernelInfoCollection, kernelInfos);
}
Expand All @@ -264,19 +259,23 @@ internal static void MergeKernelInfos(KernelInfoCollection destination, KernelIn
destination.AddRange(source.Where(ki => added.Add(ki.Name)));
}

internal static bool TryGetKernelInfoFromMetadata(
public bool TryGetKernelInfosFromMetadata(
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like a new public API. I don't remember adding this during the sync - was this exposed intentionally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We discussed this during the sync, that there's currently no way to get the kernel info using the public API, and that it's very useful to have.

[NotNullWhen(true)] out KernelInfoCollection? kernelInfos) =>
TryGetKernelInfosFromMetadata(Metadata, out kernelInfos);

internal static bool TryGetKernelInfosFromMetadata(
IDictionary<string, object>? metadata,
[NotNullWhen(true)] out KernelInfoCollection? kernelInfo)
[NotNullWhen(true)] out KernelInfoCollection? kernelInfos)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be inlined into the above non-static overload (with all callers switched to calling the non-static overload)? Or can this be made private?

Please ignore the above if we have more callers for the static overload outside the current class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The static method has internal callers.

{
if (metadata is not null)
{
if (metadata.TryGetValue("kernelInfo", out var kernelInfoObj))
{
if (kernelInfoObj is JsonElement kernelInfoJson &&
JsonSerializer.Deserialize<KernelInfoCollection>(kernelInfoJson, JsonSerializerOptions) is
kernelInfoJson.Deserialize<KernelInfoCollection>(JsonSerializerOptions) is
{ } kernelInfoDeserialized)
{
kernelInfo = kernelInfoDeserialized;
kernelInfos = kernelInfoDeserialized;
return true;
}

Expand Down Expand Up @@ -319,15 +318,15 @@ internal static bool TryGetKernelInfoFromMetadata(
deserializedKernelInfo.Add(new KernelInfo(name, language, aliases));
}
}
kernelInfo = deserializedKernelInfo;
kernelInfos = deserializedKernelInfo;
return true;
}
}
}

if (kernelInfoObj is KernelInfoCollection kernelInfoCollection)
{
kernelInfo = kernelInfoCollection;
kernelInfos = kernelInfoCollection;
return true;
}
}
Expand All @@ -338,17 +337,17 @@ internal static bool TryGetKernelInfoFromMetadata(
{
case KernelInfoCollection kernelInfoCollection:

kernelInfo = kernelInfoCollection;
kernelInfos = kernelInfoCollection;
return true;

case IDictionary<string, object> dotnetInteractiveDict:
{
kernelInfo = new();
kernelInfos = new();

if (dotnetInteractiveDict.TryGetValue("defaultKernelName", out var nameObj) &&
nameObj is string name)
{
kernelInfo.DefaultKernelName = name;
kernelInfos.DefaultKernelName = name;
}

return true;
Expand All @@ -364,7 +363,7 @@ internal static bool TryGetKernelInfoFromMetadata(
if (kernelspecDict.TryGetValue("language", out var languageObj) &&
languageObj is string defaultLanguage)
{
kernelInfo = new KernelInfoCollection
kernelInfos = new KernelInfoCollection
{
DefaultKernelName = defaultLanguage
};
Expand All @@ -375,7 +374,7 @@ internal static bool TryGetKernelInfoFromMetadata(
}

// check if a KernelInfoCollection was directly serialized into the metadata
kernelInfo = default;
kernelInfos = default;
return false;
}

Expand Down
9 changes: 6 additions & 3 deletions src/Microsoft.DotNet.Interactive.Documents/Jupyter/InputCellMetadata.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,15 @@ namespace Microsoft.DotNet.Interactive.Documents.Jupyter;

public class InputCellMetadata
{
public InputCellMetadata(string? kernelName = null, string ? language = null)
public InputCellMetadata(string? kernelName = null, string? language = null)
{
KernelName = kernelName;
Language = language;
}

[JsonPropertyName("kernelName")] public string? KernelName { get; }
[JsonPropertyName("language")] public string? Language { get; }
[JsonPropertyName("kernelName")]
public string? KernelName { get; }

[JsonPropertyName("language")]
public string? Language { get; }
}
2 changes: 1 addition & 1 deletion src/Microsoft.DotNet.Interactive.Documents/Jupyter/InteractiveDocumentExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

namespace Microsoft.DotNet.Interactive.Documents.Jupyter;

public static class InteractiveDocumentExtensions
internal static class InteractiveDocumentExtensions
{
public static InteractiveDocument WithJupyterMetadata(
this InteractiveDocument document,
Expand Down
9 changes: 7 additions & 2 deletions ...crosoft.DotNet.Interactive.Documents/Jupyter/InteractiveDocumentOutputElementConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,16 @@ public override InteractiveDocumentOutputElement Read(
break;

case "execute_result":
var returnValueElement = new ReturnValueElement(data ?? new Dictionary<string, object>())
var returnValueElement = new ReturnValueElement
{
ExecutionOrder = executionCount ?? 0
};


if (data is not null)
{
returnValueElement.Data.MergeWith(data);
}

if (metadata is not null)
{
returnValueElement.Metadata.MergeWith(metadata);
Expand Down
35 changes: 8 additions & 27 deletions src/Microsoft.DotNet.Interactive.Documents/Jupyter/Notebook.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ namespace Microsoft.DotNet.Interactive.Documents.Jupyter;

public static class Notebook
{
private static readonly Encoding _encoding = new UTF8Encoding(false);

static Notebook()
{
JsonSerializerOptions = new JsonSerializerOptions
Expand All @@ -33,12 +35,7 @@ static Notebook()
};
}

public static JsonSerializerOptions JsonSerializerOptions { get; }

public const string MetadataNamespace = "dotnet_interactive";
public const string PolyglotMetadataNamespace = "polyglot_notebook";

public static Encoding Encoding => new UTF8Encoding(false);
internal static JsonSerializerOptions JsonSerializerOptions { get; }

public static InteractiveDocument Parse(
string json,
Expand All @@ -59,24 +56,11 @@ public static InteractiveDocument Read(
Stream stream,
KernelInfoCollection kernelInfos)
{
using var reader = new StreamReader(stream, Encoding);
using var reader = new StreamReader(stream, _encoding);
var content = reader.ReadToEnd();
return Parse(content, kernelInfos);
}

public static void Write(InteractiveDocument document, Stream stream)
{
using var writer = new StreamWriter(stream, Encoding, 1024, true);
Write(document, writer);
writer.Flush();
}

public static void Write(InteractiveDocument document, Stream stream, KernelInfoCollection kernelInfos)
{
InteractiveDocument.MergeKernelInfos(document, kernelInfos);
Write(document, stream);
}

public static string ToJupyterJson(
this InteractiveDocument document,
string? defaultLanguage = null)
Expand All @@ -97,15 +81,12 @@ public static string ToJupyterJson(
return singleSpaceIndentedJson;
}

public static void Write(InteractiveDocument document, TextWriter writer)
public static void Write(InteractiveDocument document, Stream stream, KernelInfoCollection kernelInfos)
{
InteractiveDocument.MergeKernelInfos(document, kernelInfos);
using var writer = new StreamWriter(stream, _encoding, 1024, true);
var content = document.ToJupyterJson();
writer.Write(content);
writer.Flush();
}
public static void Write(InteractiveDocument document, TextWriter writer, KernelInfoCollection kernelInfos)
{
InteractiveDocument.MergeKernelInfos(document, kernelInfos);
Write(document, writer);
}

}
2 changes: 1 addition & 1 deletion src/Microsoft.DotNet.Interactive.Documents/KernelInfoCollection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public KernelInfoCollection Clone()
return clone;
}

public bool TryGetByAlias(string alias, out KernelInfo info)
internal bool TryGetByAlias(string alias, out KernelInfo info)
{
return _kernelInfoByNameOrAlias!.TryGetValue(alias, out info);
}
Expand Down
8 changes: 1 addition & 7 deletions src/Microsoft.DotNet.Interactive.Documents/ReturnValueElement.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,14 @@
// Copyright (c) .NET Foundation and contributors. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
using System.Collections.Generic;

namespace Microsoft.DotNet.Interactive.Documents;

public class ReturnValueElement : InteractiveDocumentOutputElement, IDataElement
{
public ReturnValueElement(IDictionary<string, object>? data)
{
Data = data ?? throw new ArgumentNullException(nameof(data));
}
public IDictionary<string, object> Data { get; } = new Dictionary<string, object>();

public IDictionary<string, object> Data { get; }

public int ExecutionOrder { get; set; }

public IDictionary<string, object> Metadata { get; } = new Dictionary<string, object>();
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.DotNet.Interactive.Documents/TextElement.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ namespace Microsoft.DotNet.Interactive.Documents;

public class TextElement : InteractiveDocumentOutputElement
{
public TextElement(string? text, string? name)
public TextElement(string? text, string? name = "stdout")
{
Text = text ?? "";
Name = name ?? "stdout";
Expand Down
8 changes: 6 additions & 2 deletions src/Microsoft.DotNet.Interactive.Jupyter/JupyterRequestContextExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,17 @@ public static class JupyterRequestContextExtensions
public static string GetKernelName(this JupyterRequestContext context)
{
string kernelName = null;
if (context.JupyterRequestMessageEnvelope.MetaData.TryGetValue(Notebook.MetadataNamespace, out var candidateDotnetMetadata) &&
if (context.JupyterRequestMessageEnvelope.MetaData.TryGetValue(
"dotnet_interactive",
out var candidateDotnetMetadata) &&
candidateDotnetMetadata is InputCellMetadata dotnetMetadata)
{
kernelName = dotnetMetadata.Language;
}

if (context.JupyterRequestMessageEnvelope.MetaData.TryGetValue(Notebook.PolyglotMetadataNamespace, out var candidatePolyglotMetadata) &&
if (context.JupyterRequestMessageEnvelope.MetaData.TryGetValue(
"polyglot_notebook",
out var candidatePolyglotMetadata) &&
candidatePolyglotMetadata is InputCellMetadata polyglotMetadata)
{
kernelName = polyglotMetadata.KernelName;
Expand Down