Backport docs for System.Numerics.Vectors #47725

carlossanlop · 2021-02-01T22:11:22Z

Backporting public API documentation from MS Docs into triple slash comments for the System.Numerics.Vectors namespace.

This namespace was interesting for this experiment because all the source files live in System.Private.CoreLib, so the porting tool had to make sure to read the System.Numerics.Vectors.csproj dependent projects and find the source code files in them.

The full description of the purpose of this PR is described in the issue #44969 - Pilot a new process that extracts API documentation from source code under the section "Bring documentation from Docs to triple slash".

I ran the porting tool using this command:

DocsPortingTool.exe \
-CsProj D:\runtime\src\libraries\System.Numerics.Vectors\src\System.Numerics.Vectors.csproj \
-Docs D:\dotnet-api-docs\xml \
-Save true \
-Direction ToTripleSlash \
-IncludedAssemblies System.Numerics \
-IncludedNamespaces System.Numerics

Please compare the ported docs with their source files in dotnet-api-docs:
https://github.com/dotnet/dotnet-api-docs/tree/master/xml/System.Numerics

ghost · 2021-02-01T22:11:25Z

Tagging subscribers to this area: @tannergooding, @pgovind
See info in area-owners.md if you want to be subscribed.

Issue Details

Backporting public API documentation from MS Docs into triple slash comments for the System.Numerics.Vectors namespace.

This namespace was interesting for this experiment because all the source files live in System.Private.CoreLib, so the porting tool had to make sure to read the System.Numerics.Vectors.csproj dependent projects and find the source code files in them.

The full description of the purpose of this PR is described in the issue #44969 - Pilot a new process that extracts API documentation from source code under the section "Bring documentation from Docs to triple slash".

I ran the porting tool using this command:

DocsPortingTool.exe \
-CsProj D:\runtime\src\libraries\System.Numerics.Vectors\src\System.Numerics.Vectors.csproj \
-Docs D:\dotnet-api-docs\xml \
-Save true \
-Direction ToTripleSlash \
-IncludedAssemblies System.Numerics \
-IncludedNamespaces System.Numerics

Author:	carlossanlop
Assignees:	carlossanlop
Labels:	`area-System.Numerics`, `documentation`
Milestone:	6.0.0

carlossanlop · 2021-02-01T22:14:34Z

Need to refresh this branch. There are some recent nullability changes causing a conflict.
Edit: Fixed with a force push below.

safern · 2021-02-01T22:15:26Z

src/libraries/System.Numerics.Vectors/System.Numerics.Vectors.sln

@@ -1,4 +1,8 @@
-Microsoft Visual Studio Solution File, Format Version 12.00


I guess these changes to the sln file are not intentional?

Visual Studio made those changes automatically (first commit in the PR).

I think these changes occur when a newer version of VS opens the sln. AFAIK, reverting these changes work, so that's what I've always done.

It's kind of annoying having to revert the changes every time I have to submit a new commit. @safern would I break something if I keep the sln changes?

I'll keep this unless it causes any issues.

src/libraries/System.Numerics.Vectors/src/System.Numerics.Vectors.csproj

safern · 2021-02-01T22:18:17Z

This namespace was interesting for this experiment because all the source files live in System.Private.CoreLib, so the porting tool had to make sure to read the System.Numerics.Vectors.csproj dependent projects and find the source code files in them.

For implementation that lives in System.Private.CoreLib, we should run the tool with RuntimeFlavor=Mono so that specific API that is exposed via the Mono S.P.CoreLib if any, gets it's docs ported.

src/libraries/System.Private.CoreLib/src/System/Numerics/Matrix3x2.cs

src/libraries/System.Private.CoreLib/src/System/Numerics/Matrix4x4.cs

src/libraries/System.Private.CoreLib/src/System/Numerics/Matrix3x2.cs

gewarren · 2021-02-03T17:45:33Z

src/libraries/System.Private.CoreLib/src/System/Numerics/Matrix3x2.cs

@@ -7,7 +7,8 @@

 namespace System.Numerics
 {
-    /// <summary>A structure encapsulating a 3x2 matrix.</summary>
+    /// <summary>Represents a 3x2 matrix.</summary>
+    /// <remarks>[!INCLUDE[vectors-are-rows-paragraph](~/includes/system-numerics-vectors-are-rows.md)]</remarks>


I'm not 100% sure if the INCLUDE syntax works in non-Markdown sections.

@gewarren that's a good point. In the first PR, I was asked to remove the CDATA since it was not providing any value in triple slash comments, was using a lot of unnecessary space and polluting the code visually.

So I have a question for you to help address this: When we send intellisense xmls to the Docs build system, is it smart enough to automatically wrap non-empty remarks (including remarks that say To be added.) with these tags?:

<format type="text/markdown><![CDATA[ ... ]]></format>

If not, then we have two options:

A) Convert markdown links like these to valid xml. Do you know what's the non-markdown syntax for a link to an md file? And do you know if the Docs build system will be able to read that correctly and render the md document properly?
B) Talk to the Docs dev team to request converting any remarks we send to them to markdown (if they don't do it yet).

Thoughts?

Another thing to consider is that the xmls that Roslyn will produce after the source of truth are comments in source code, we will be shipping that as part of the intellisense xml files right? How will this look when a customer hovers over an API on VS and VS takes the info for that API from comments with these includes or [CDATA]?

AFAIK, remarks do not show up in VS intellisense in any way.

Do you know what's the non-markdown syntax for a link to an md file?

I don't know. And the thing is, you don't just want a link, you actually want to import the contents of the linked file.

When we send intellisense xmls to the Docs build system, is it smart enough to automatically wrap non-empty remarks?

No, it does not. See for example here.

@gewarren to address this problem, we can keep the CDATA section for remarks that require to show an include.

That's what the machinelearning repo does for its linked code snippets. See https://github.com/dotnet/machinelearning/blob/master/src/Microsoft.ML.FastTree/TreeTrainersCatalog.cs#L30.

src/libraries/System.Private.CoreLib/src/System/Numerics/Matrix3x2.cs

gewarren · 2021-02-03T17:57:22Z

src/libraries/System.Private.CoreLib/src/System/Numerics/Matrix3x2.cs

        /// <returns>The product matrix.</returns>
+        /// <remarks>The <see cref="System.Numerics.Matrix3x2.op_Multiply" /> method defines the operation of the multiplication operator for <see cref="System.Numerics.Matrix3x2" /> objects.</remarks>


The crefs need to be prefixed with a P: for property, M: for method, etc. If this reference is referring to both overloads of the * operator, then I think the prefix is Overload:.

Doesn't C# add these automatically? If not should it be updated to do so?

Oh yes, you are right. So this should be fine then. https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/xmldoc/cref-attribute

There's even a code analysis rule that enforces not setting a prefix - https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1200.

@gewarren There is a bug preventing me from indicating a method group in triple slash comments (the equivalent of Overload: as you mentioned). There is an open issue tracking this problem in the csharplang repo: dotnet/csharplang#320

Would you mind helping me increase the priority/visibility of the csharplang issue? I am currently running the tool in other larger assemblies, and this is going to become a very common problem when porting types with lots of overloads (Process, FileStream are giving me a headache).

gewarren · 2021-02-03T18:05:13Z

src/libraries/System.Private.CoreLib/src/System/Numerics/Quaternion.cs

@@ -8,28 +8,32 @@
 namespace System.Numerics
 {
    /// <summary>Represents a vector that is used to encode three-dimensional physical rotations.</summary>
+    /// <remarks>The <see cref="System.Numerics.Quaternion" /> structure is used to efficiently rotate an object about the (x,y,z) vector by the angle theta, where:
+    /// ```


Not sure about using ``` in XML. Also, it should have a code language.

Good catch. These are the cases that we need to manually move to a file in dotnet-api-docs, then link to it here.

Found a bunch of these and I have converted them either to remarks with format + cdata, or to <c></c>.

src/libraries/System.Private.CoreLib/src/System/Numerics/Vector.cs

carlossanlop · 2021-02-10T05:00:01Z

@gewarren @tannergooding @pgovind I marked this draft as ready for review. I addressed all your comments and suggestions and made sure the remarks with special markdown data show their information properly. Can you please take a look?

safern · 2021-02-10T18:43:56Z

src/libraries/System.Numerics.Vectors/System.Numerics.Vectors.sln

-		{875DAA4F-1BC0-4EE2-9A04-EDD2FC5C773F} = {7A13467C-8469-4562-B00F-C4D61E8A1855}
-		{BD5C16FE-CAAA-45CB-91F3-C66A2493C518} = {7A13467C-8469-4562-B00F-C4D61E8A1855}
+	GlobalSection(SharedMSBuildProjectFiles) = preSolution
+		..\System.Private.CoreLib\src\System.Private.CoreLib.Shared.projitems*{634a3b2b-09f5-4810-b630-ade4d36c47df}*SharedItemsImports = 5


I don't understand why this is being added? Did you change this on VS?

No, I didn't modify the sln file myself. Visual Studio did this. I'll revert the sln file change.

@safern reverted.

carlossanlop · 2021-02-11T00:58:27Z

Learnings from this PR:

Overloads can't be shown in triple slash comments. This is a bug, and it is tracked by Ability to reference a method group in XML comment csharplang#320 .
When remarks contain a hyperlink or an include to an example, the whole element needs to be transformed to markdown (wrapped by <format type="text/markdown"><![CDATA[ ... ]]></format>).

carlossanlop · 2021-02-12T05:28:49Z

The two CI failures are unrelated to my change. I already tried re-running both legs 4 times and the failure is consistent.

The Libraries Test Run release coreclr windows x64 Debug CI leg is failing with this:

ERROR: helix-scripts 20191204.1 has requirement azure-core==1.0.0, but you'll have azure-core 1.7.0 which is incompatible.
ERROR: helix-scripts 20191204.1 has requirement azure-storage-blob==12.0.0, but you'll have azure-storage-blob 12.5.0 which is incompatible.
ERROR: helix-scripts 20191204.1 has requirement certifi==2018.11.29, but you'll have certifi 2020.6.20 which is incompatible.

And the Installer Build and Test coreclr windows_x64 Release CI leg with this:

D:\workspace\_work\1\s\.packages\microsoft.dotnet.sharedframework.sdk\6.0.0-beta.21105.5\targets\sharedfx.targets(273,5): error : The following files are missing entries in the templated manifest: [D:\workspace\_work\1\s\src\installer\pkg\sfx\Microsoft.NETCore.App\Microsoft.NETCore.App.Runtime.sfxproj]
D:\workspace\_work\1\s\.packages\microsoft.dotnet.sharedframework.sdk\6.0.0-beta.21105.5\targets\sharedfx.targets(273,5): error : pgort140.dll. Add these file names with extensions to the 'PlatformManifestFileEntry' item group for the runtime pack and corresponding ref pack to include them in the platform manifest. [D:\workspace\_work\1\s\src\installer\pkg\sfx\Microsoft.NETCore.App\Microsoft.NETCore.App.Runtime.sfxproj]
##[error].packages\microsoft.dotnet.sharedframework.sdk\6.0.0-beta.21105.5\targets\sharedfx.targets(273,5): error : (NETCORE_ENGINEERING_TELEMETRY=Build) The following files are missing entries in the templated manifest:
pgort140.dll. Add these file names with extensions to the 'PlatformManifestFileEntry' item group for the runtime pack and corresponding ref pack to include them in the platform manifest.

Build FAILED.

carlossanlop added documentation Documentation bug or enhancement, does not impact product or test code area-System.Numerics labels Feb 1, 2021

carlossanlop added this to the 6.0.0 milestone Feb 1, 2021

carlossanlop requested review from jeffhandley, pgovind, tannergooding and safern February 1, 2021 22:11

carlossanlop self-assigned this Feb 1, 2021

carlossanlop mentioned this pull request Feb 1, 2021

Libraries APIs are fully documented using efficient workflows #44969

Open

16 tasks

safern reviewed Feb 1, 2021

View reviewed changes

carlossanlop added 4 commits February 1, 2021 14:15

Unavoidable refresh by Visual Studio of the csproj file.

a82832e

Backport Matrix3x2

770136b

Backport Matrix4x4

003afb2

Backport Plane

341cd5d

safern reviewed Feb 1, 2021

View reviewed changes

src/libraries/System.Numerics.Vectors/src/System.Numerics.Vectors.csproj Show resolved Hide resolved

carlossanlop added 11 commits February 1, 2021 14:17

Backport Quaternion

c9b1b43

Backport Vector

f13c561

Backport Vector2

8ba447a

Backport Vector3

b19650f

Backport Vector4

73d12fc

Manually fix Plane < and >

d558fec

Manually fix Vector wrong param names

9892f9e

Manually fix Vector2 < and >

110747b

Manually fix Vector3 < and >

eb53a7f

Manually fix Vector4 < and >

17f0fb9

Add GenerateDocumentationFile to csproj

23e4e67

tannergooding reviewed Feb 2, 2021

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/Numerics/Matrix3x2.cs Outdated Show resolved Hide resolved

tannergooding reviewed Feb 2, 2021

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/Numerics/Matrix3x2.cs Outdated Show resolved Hide resolved

tannergooding reviewed Feb 2, 2021

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/Numerics/Matrix4x4.cs Show resolved Hide resolved

pgovind reviewed Feb 2, 2021

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/Numerics/Matrix3x2.cs Show resolved Hide resolved

carlossanlop requested a review from gewarren February 2, 2021 21:32

gewarren reviewed Feb 3, 2021

View reviewed changes

carlossanlop added 8 commits February 9, 2021 20:22

Matrix3x2 adjust remarks.

92bf4da

Quaternion address remarks.

2703bb3

Plane adjust remarks.

d1af7c2

Vector4 adjust remarks.

98fa118

Matrix4x4 adjust remarks.

faee5b3

Vector2 adjust remarks.

fbe8c4b

Vector3 adjust remarks.

c764c06

Address tannergooding and gewarren suggestions.

4095f29

carlossanlop marked this pull request as ready for review February 10, 2021 04:59

carlossanlop added 2 commits February 9, 2021 21:01

Matrix4x4 tries to invert.

82e533c

Bring back GenerateDocumentationFile in csproj

370322c

safern reviewed Feb 10, 2021

View reviewed changes

tannergooding approved these changes Feb 10, 2021

View reviewed changes

Revert sln change

e760180

carlossanlop merged commit adf616b into dotnet:master Feb 12, 2021

carlossanlop deleted the NumericsVectorsBackport branch February 12, 2021 05:30

carlossanlop linked an issue Mar 2, 2021 that may be closed by this pull request

System.Numerics: Backport MS Docs documentation to triple slash #48968

Closed

carlossanlop mentioned this pull request Mar 2, 2021

System.Numerics: Backport MS Docs documentation to triple slash #48968

Closed

ghost locked as resolved and limited conversation to collaborators Mar 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Backport docs for System.Numerics.Vectors #47725

Backport docs for System.Numerics.Vectors #47725

carlossanlop commented Feb 1, 2021 •

edited

Loading

ghost commented Feb 1, 2021

carlossanlop commented Feb 1, 2021 •

edited

Loading

safern Feb 1, 2021

carlossanlop Feb 1, 2021

pgovind Feb 2, 2021

carlossanlop Feb 5, 2021

carlossanlop Feb 10, 2021

safern commented Feb 1, 2021

gewarren Feb 3, 2021

carlossanlop Feb 5, 2021

safern Feb 5, 2021

carlossanlop Feb 8, 2021

gewarren Feb 9, 2021

carlossanlop Feb 9, 2021

gewarren Feb 9, 2021

gewarren Feb 3, 2021

tannergooding Feb 3, 2021

gewarren Feb 3, 2021

gewarren Feb 5, 2021

carlossanlop Feb 10, 2021

gewarren Feb 10, 2021

gewarren Feb 3, 2021

carlossanlop Feb 5, 2021

carlossanlop Feb 10, 2021

carlossanlop commented Feb 10, 2021

safern Feb 10, 2021

carlossanlop Feb 11, 2021

carlossanlop Feb 11, 2021

carlossanlop commented Feb 11, 2021

carlossanlop commented Feb 12, 2021 •

edited

Loading

		@@ -1,4 +1,8 @@
		Microsoft Visual Studio Solution File, Format Version 12.00

		/// <returns>The product matrix.</returns>
		/// <remarks>The <see cref="System.Numerics.Matrix3x2.op_Multiply" /> method defines the operation of the multiplication operator for <see cref="System.Numerics.Matrix3x2" /> objects.</remarks>

Backport docs for System.Numerics.Vectors #47725

Backport docs for System.Numerics.Vectors #47725

Conversation

carlossanlop commented Feb 1, 2021 • edited Loading

ghost commented Feb 1, 2021

carlossanlop commented Feb 1, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

safern commented Feb 1, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlossanlop commented Feb 10, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlossanlop commented Feb 11, 2021

carlossanlop commented Feb 12, 2021 • edited Loading

carlossanlop commented Feb 1, 2021 •

edited

Loading

carlossanlop commented Feb 1, 2021 •

edited

Loading

carlossanlop commented Feb 12, 2021 •

edited

Loading