-
Notifications
You must be signed in to change notification settings - Fork 1.9k
XML strings for the documentation should live outside of the src code, in xml files. #510
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
Conversation
…les. For the summary text that is common between several learners, the examples will be added on a separate node. An example of how that will look like is in the LogisticRegressionBinaryClassifier and LogisticRegressionClassifier.
Thanks for doing this @sfilipi ! So what are best practices here? I might have expected that any XML include files would be in the same directory or some subdirectory as the project with the relevant code, as opposed to some other location way off in the hinterlands not even in the "source" files. Secondary reason for my preference is that it might be nice if |
That is, I'd expect the FastTree XML docs to be somewhere in the FastTree project, LightGBM in lightGBM project, etc... rather than having master files where all documentation lives. I'm not sure I consider these massive files maintainable. In reply to: 403604163 [](ancestors = 403604163) |
Was on the fence between single file, per component type, and per-project files. In reply to: 403605251 [](ancestors = 403605251,403604163) |
Hmmmm. :) Sorry what votes? In reply to: 403606882 [](ancestors = 403606882,403605251,403604163) |
The OSX failures are unrelated to my changes. They fail to retrieve some of the nuget dependancies. #Resolved |
@@ -20,6 +20,7 @@ public interface IFastTreeTrainerFactory : IComponentFactory<ITrainer> | |||
{ | |||
} | |||
|
|||
/// <include file='./XMLDoc.xml' path='docs/members/member[@name="FastTree"]/*' /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
XMLDoc [](start = 25, length = 6)
I might be being a little OCD, but the redundancy of the name XMLDoc.xml
just drives me absolutely bonkers. Maybe just plain old doc.xml
(lower case doc
)? #Resolved
src/Microsoft.ML.FastTree/XMLDoc.xml
Outdated
</summary> | ||
<remarks> | ||
<para> | ||
FastTrees is an efficient implementation of the <a href='https://arxiv.org/abs/1505.01866'>MART</a> gradient boosting algorithm. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FastTrees [](start = 10, length = 9)
FastTree, not FastTrees. #Resolved
@@ -529,9 +529,9 @@ public sealed class EntryPointAttribute : Attribute | |||
public string ShortName { get; set; } | |||
|
|||
/// <summary> | |||
/// Remarks on the Entry Point, for more extensive XML documentation on the C#API | |||
/// The path to the XML documentation on the C#API component |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C#API [](start = 57, length = 5)
C# API?
</summary> | ||
<remarks> | ||
<para> | ||
FastTree is an efficient implementation of the <a href='https://arxiv.org/abs/1505.01866'>MART</a> gradient boosting algorithm. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MART [](start = 100, length = 4)
This is a bit confusing, We say MART but the paper is an improved version DART. @tfinley@gmail.com do you know what is the correct one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we use the acronym "DART" I think we need to have a link to that paper, and also explain what is meant by that. I think it's fine to just say MART honestly. The DART capabilities could perhaps be mentioned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @sfilipi !
…, in xml files. (dotnet#510) * Moving from xml strings to having the documentation details in xml files. For the summary text that is common between several learners, the examples will be added on a separate node. An example of how that will look like is in the LogisticRegressionBinaryClassifier and LogisticRegressionClassifier. * fixing the aftermath of renaming the XML file. * removing the Desc from the EntryPoint attribute is a bad idea. * removing the XML docs from the doc folder, and added them under the respective projects. * Some OS get picky about casing. * file name should be vanilla * Fixing comment
…, in xml files. (dotnet#510) * Moving from xml strings to having the documentation details in xml files. For the summary text that is common between several learners, the examples will be added on a separate node. An example of how that will look like is in the LogisticRegressionBinaryClassifier and LogisticRegressionClassifier. * fixing the aftermath of renaming the XML file. * removing the Desc from the EntryPoint attribute is a bad idea. * removing the XML docs from the doc folder, and added them under the respective projects. * Some OS get picky about casing. * file name should be vanilla * Fixing comment
Resolves #477 by moving the strings with the XML remarks, examples etc into a separate XML file.
Now the actual classes and their C# Api counterparts share the path to the XML node that contains the documentation and the respective example.
On this PR, there are only two examples that are separate from the rest of the descriptive xml: the LogisticRegressionBinaryClassifier and LogisticRegressionClassifier, to give an idea of the infrastructure.
The rest of the examples are coming in the next PR, together with the rest of the documentation.