Skip to content

Fixes #591: typos, adding the type attribute to lists, and moving the name attribute for some examples #592

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

Merged
merged 4 commits into from
Jul 28, 2018

Conversation

sfilipi
Copy link
Member

@sfilipi sfilipi commented Jul 27, 2018

Testing the documentation in the doc.microsoft.com staging environment, i noticed a few typos, unescaped apostrophes, and missing xml tag attributes.

Fixes #591: typos, adding the type to lists, and fixing the name attribute in OGD and Poisson

@sfilipi sfilipi requested review from shauheen, TomFinley and Zruty0 July 27, 2018 16:30
Copy link
Contributor

@TomFinley TomFinley left a comment

Choose a reason for hiding this comment

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

Thank you @sfilipi !

@@ -13,8 +13,8 @@
and an option to update the weight vector using the average of the vectors seen over time (averaged argument is set to True by default).
</remarks>
</member>
<example>
<example name="OGD">
<example name="OGD">
Copy link
Contributor

@Zruty0 Zruty0 Jul 28, 2018

Choose a reason for hiding this comment

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

example [](start = 5, length = 7)

is this not an error to have nested examples? #Pending

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea I see it's used everywhere. I wonder why though?


In reply to: 205926076 [](ancestors = 205926076)

Copy link
Member Author

@sfilipi sfilipi Jul 28, 2018

Choose a reason for hiding this comment

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

it is legal in xml.
The internal example node is needed by the documentation to display the snippet as an example.

I needed a named wrapper to get the node, and ...just called it example.

this is how they are being included:

I don't need to include them with the /* notation, i can just include the named node itself, but i wasn't sure if the docs tools would tolerate the name attribute on it.
But with both you and Ivan commenting on it, though, might be worth to try it, and if they are robust to it, clean this two level example.

In my TODO list.


In reply to: 205926101 [](ancestors = 205926101,205926076)

@@ -13,8 +13,8 @@
and an option to update the weight vector using the average of the vectors seen over time (averaged argument is set to True by default).
</remarks>
</member>
<example>
<example name="OGD">
<example name="OGD">
Copy link
Contributor

@Ivanidzo4ka Ivanidzo4ka Jul 28, 2018

Choose a reason for hiding this comment

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

example name="OGD" [](start = 5, length = 18)

out of curiosity, is order of having name / not having name matter? In AP example you have and here you have #Pending

Copy link
Member Author

Choose a reason for hiding this comment

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

it depends how you include the node in the code.

see the long answer to Pete's comment. Should clarify it.
Note the * notation:


In reply to: 205926135 [](ancestors = 205926135)

@@ -7,8 +7,7 @@
Encodes the categorical variable with hash-based encoding.
</summary>
<remarks>
CategoricalHashOneHotVectorizer converts a categorical value into an indicator array by hashing the
value and using the hash as an index in the bag.
CategoricalHashOneHotVectorizer converts a categorical value into an indicator array by hashing the value and using the hash as an index in the bag.
Copy link
Contributor

@Zruty0 Zruty0 Jul 28, 2018

Choose a reason for hiding this comment

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

the value [](start = 104, length = 9)

why removing the newline here, but not, say, in line 33? #Pending

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not sure why the text is broken in newlines identically how my text is structured for the page corresponding to the CategoricalHashOneHotVectorizer, and it is flowing differently for the CategoricalOneHotVectorizer, disregarding line indentations in the XML.

Could be because the text in the other one is inside the tags; and we should always use the para tags to keep the convenience formatting in the xml.

links if curious:
https://review.docs.microsoft.com/en-us/dotnet/api/microsoft.ml.transforms.categoricalonehotvectorizer?view=ml-dotnet&branch=smoke-test-preview

https://review.docs.microsoft.com/en-us/dotnet/api/microsoft.ml.transforms.categoricalhashonehotvectorizer?view=ml-dotnet&branch=smoke-test-preview


In reply to: 205926140 [](ancestors = 205926140)

Copy link
Contributor

@Zruty0 Zruty0 left a comment

Choose a reason for hiding this comment

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

:shipit:

@sfilipi sfilipi merged commit 502e422 into dotnet:master Jul 28, 2018
@sfilipi sfilipi deleted the docFormattingFixes branch July 28, 2018 00:58
@@ -33,16 +32,16 @@
<para>The CategoricalOneHotVectorizer transform passes through a data set, operating on text columns, to
build a dictionary of categories.
For each row, the entire text string appearing in the input column is defined as a category.
The output of this transform is an indicator vector.
The output of this transform is an indicator vector.</para>
Each slot in this vector corresponds to a category in the dictionary, so its length is the size of the built dictionary.
Copy link
Member

Choose a reason for hiding this comment

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

📝 This line should be in a paragraph element.

@@ -157,28 +157,28 @@
</summary>
<remarks>
In machine learning​ it is a pretty common and powerful approach to utilize the already trained model in the process of defining features.
<para>One such example would be the use of model's scores as features to downstream models. For example, we might run clustering on the original features,
<para>One such example would be the use of model&apos;s scores as features to downstream models. For example, we might run clustering on the original features,
Copy link
Member

Choose a reason for hiding this comment

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

❔ Why is this required? It's awkward to read and seems like it would be easy to regress.

<code language="csharp">
pipeline.Add(new MissingValueHandler(&quot;FeatureCol&quot;, &quot;CleanFeatureCol&quot;)
{
ReplaceWith = NAHandleTransformReplacementKind.Mean
Copy link
Member

Choose a reason for hiding this comment

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

💡 Should likely be indented

<example name="NAHandle">
<example>
<code language="csharp">
pipeline.Add(new MissingValueHandler(&quot;FeatureCol&quot;, &quot;CleanFeatureCol&quot;)
Copy link
Member

Choose a reason for hiding this comment

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

❔ Why is the &quot; escape needed? I would only expect this if it appeared as the value of an XML attribute.

It can be changed to true for known length vectors, but it results in an error if changed to false for variable length vectors.
</para>
</remarks>
<seealso cref=" Microsoft.ML.Runtime.Data.MetadataUtils.Kinds.HasMissingValues"/>
Copy link
Member

Choose a reason for hiding this comment

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

❔ Is this missing a T: prefix? It's unclear why the others are given in full form but I noticed this one is different.

codemzs pushed a commit to codemzs/machinelearning that referenced this pull request Aug 1, 2018
…ng the name attribute for some examples (dotnet#592)

* Fixes issue 591: typos, adding the type to lists, and fixing the name attribute in OGD and Poisson

* getting just the content under the memeber node, not the member itself.

* merging from master
@ghost ghost locked as resolved and limited conversation to collaborators Mar 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix documentation formatting
5 participants