add projects capability in CodeGenerator - #5002

Merged
harishsk merged 1 commit into
dotnet:masterfrom
LittleLittleCloud:u/xiaoyun/addProjectCapabilities
Apr 7, 2020
Merged

add projects capability in CodeGenerator#5002
harishsk merged 1 commit into
dotnet:masterfrom
LittleLittleCloud:u/xiaoyun/addProjectCapabilities

Conversation

@LittleLittleCloud

@LittleLittleCloudLittleLittleCloud commented Apr 6, 2020

Copy link
Copy Markdown
Member

if generated by ModelBuilder, will add
<ProjectCapability Include="ModelBuilderGenerated" />
else
<ProjectCapability Include="MLNETCLIGenerated" />
to .Model and .Console project

@LittleLittleCloud
LittleLittleCloud requested a review from a team as a code ownerApril 6, 2020 18:31
justinormont
justinormont reviewed Apr 6, 2020
View reviewed changes
Comment threadsrc/Microsoft.ML.CodeGenerator/Templates/Console/ModelProject.tt
<ProjectCapability Include="MLNETCLIGenerated" />
<#}else{#>
<ProjectCapability Include="ModelBuilderGenerated" />
<#}#>

@justinormontjustinormontApr 6, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What's diverging between CLI and MB that we need MLNETCLIGenerated vs ModelBuilderGenerated?

@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

It's from our PM, we want to differentiate ModelBuilder-Generated from MLNet CLI Generated in telemetry data

@justinormontjustinormontApr 6, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sounds good.

justinormont
justinormont reviewed Apr 6, 2020
View reviewed changes
Comment threadsrc/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.cs
this.Write(" <ProjectCapability Include=\"MLNETCLIGenerated\" />\r\n");
}else{
this.Write(" <ProjectCapability Include=\"ModelBuilderGenerated\" />\r\n");
}

@justinormontjustinormontApr 6, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should modify the upstream .tt so the .cs is formatted properly here.

@justinormontjustinormontApr 6, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same w/ ModelProject.tt

@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

What you mean

@JakeRadMSFTJakeRadMSFTApr 6, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@LittleLittleCloud aren't these files generated from TT files?

@JakeRadMSFTJakeRadMSFTApr 6, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In this PR it looks like the CS files were modified directly instead of changing the TT files.

@JakeRadMSFTJakeRadMSFTApr 6, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nevermind I see them now :)

@justinormontjustinormontApr 6, 2020•
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The newly added code is currently being formatted as:

machinelearning/src/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.cs

Lines 77 to 86 in e84006b

this.Write(this.ToStringHelper.ToStringWithCulture(Namespace));
this.Write(".Model.csproj\" />\r\n </ItemGroup>\r\n\r\n <ItemGroup>\r\n");
if(Target==CSharp.GenerateTarget.Cli){
this.Write(" <ProjectCapability Include=\"MLNETCLIGenerated\" />\r\n");
}else{
this.Write(" <ProjectCapability Include=\"ModelBuilderGenerated\" />\r\n");
}
this.Write(" </ItemGroup>\r\n</Project>\r\n");
returnthis.GenerationEnvironment.ToString();
}

It should be:

this.Write(this.ToStringHelper.ToStringWithCulture(Namespace));this.Write(".Model.csproj\" />\r\n </ItemGroup>\r\n\r\n <ItemGroup>\r\n");if(Target==CSharp.GenerateTarget.Cli){this.Write(" <ProjectCapability Include=\"MLNETCLIGenerated\" />\r\n");}else{this.Write(" <ProjectCapability Include=\"ModelBuilderGenerated\" />\r\n");}this.Write(" </ItemGroup>\r\n</Project>\r\n");returnthis.GenerationEnvironment.ToString();}

The generated .cs files are created from their corresponding .tt T4 template files. The change would occur there. I left a comment on the .cs as it shows the output.

@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

@JakeRadMSFT it's auto-gen, I didn't modify those cs file a bit :)

JakeRadMSFT
JakeRadMSFT reviewed Apr 6, 2020
View reviewed changes
Comment threadsrc/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.tt
public bool IncludeRecommenderPackage {get;set;}
public string StablePackageVersion {get;set;}
public string UnstablePackageVersion {get;set;}
internal CSharp.GenerateTarget Target {get;set;}

@JakeRadMSFTJakeRadMSFTApr 6, 2020•
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

NIT: I would think of CLI vs ModelBuilder as the Source instead of Target. It's the thing causing the projects to get generated.

@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Will do

justinormont
justinormont approved these changes Apr 6, 2020
View reviewed changes

@justinormontjustinormont left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM; I'd recommend improving the code formatting: #5002 (review)

@justinormont

justinormont commented Apr 7, 2020•
edited
Loading

Copy link
Copy Markdown
Contributor

Current CI error on one Centos and one Ubuntu run is:

- ##[error]Unhandled exception. System.IO.IOException: No space left on device

@harishsk: Thoughts on how to resolve?

@harishsk

harishsk commented Apr 7, 2020

Copy link
Copy Markdown
Contributor

@harishsk: Thoughts on how to resolve?

@frank-dong-ms I believe you are already looking at this, aren't you?

@justinormont

justinormont commented Apr 7, 2020

Copy link
Copy Markdown
Contributor

Issues/PRs relating to running out of disk space on build machines:

@JakeRadMSFTJakeRadMSFT changed the title add projects capacities in CodeGeneratoradd projects capability in CodeGeneratorApr 7, 2020
@harishsk
harishsk merged commit e451c80 into dotnet:masterApr 7, 2020
@ghostghost locked as resolved and limited conversation to collaborators Mar 18, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@JakeRadMSFTJakeRadMSFTJakeRadMSFT approved these changes
+1 more reviewer
@justinormontjustinormontjustinormont approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@LittleLittleCloud@justinormont@harishsk@JakeRadMSFT
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
add projects capability in CodeGenerator - #5002
Merged
harishsk merged 1 commit into
dotnet:masterfrom 
LittleLittleCloud:u/xiaoyun/addProjectCapabilities
Apr 7, 2020
Merged
add projects capability in CodeGenerator#5002
harishsk merged 1 commit into
dotnet:masterfrom 
LittleLittleCloud:u/xiaoyun/addProjectCapabilities
Conversation
@LittleLittleCloud
@LittleLittleCloudLittleLittleCloud
commented
Apr 6, 2020

Copy link


Copy Markdown

Member
if generated by ModelBuilder, will add
<ProjectCapability Include="ModelBuilderGenerated" />

else
<ProjectCapability Include="MLNETCLIGenerated" />

to .Model and .Console project
@LittleLittleCloud
LittleLittleCloud
requested a review
from a team
as a code ownerApril 6, 2020 18:31
justinormont
justinormont
reviewed
Apr 6, 2020
View reviewed changes
Comment threadsrc/Microsoft.ML.CodeGenerator/Templates/Console/ModelProject.tt
 <ProjectCapability Include="MLNETCLIGenerated" />
<#}else{#>
 <ProjectCapability Include="ModelBuilderGenerated" />
<#}#>
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

What's diverging between CLI and MB that we need MLNETCLIGenerated vs ModelBuilderGenerated?
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

It's from our PM, we want to differentiate ModelBuilder-Generated from MLNet CLI Generated in telemetry data
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Sounds good.
justinormont
justinormont
reviewed
Apr 6, 2020
View reviewed changes
Comment threadsrc/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.cs
 this.Write(" <ProjectCapability Include=\"MLNETCLIGenerated\" />\r\n");
}else{
 this.Write(" <ProjectCapability Include=\"ModelBuilderGenerated\" />\r\n");
}
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Should modify the upstream .tt so the .cs is formatted properly here.
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Same w/ ModelProject.tt
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

What you mean
@JakeRadMSFTJakeRadMSFTApr 6, 2020

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

@LittleLittleCloud aren't these files generated from TT files?
@JakeRadMSFTJakeRadMSFTApr 6, 2020

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

In this PR it looks like the CS files were modified directly instead of changing the TT files.
@JakeRadMSFTJakeRadMSFTApr 6, 2020

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Nevermind I see them now :)
@justinormontjustinormontApr 6, 2020•

edited

Loading

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

The newly added code is currently being formatted as:
machinelearning/src/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.cs

Lines 77 to 86
in
e84006b
this.Write(this.ToStringHelper.ToStringWithCulture(Namespace));
this.Write(".Model.csproj\" />\r\n </ItemGroup>\r\n\r\n <ItemGroup>\r\n");
if(Target==CSharp.GenerateTarget.Cli){
this.Write(" <ProjectCapability Include=\"MLNETCLIGenerated\" />\r\n");
}else{
this.Write(" <ProjectCapability Include=\"ModelBuilderGenerated\" />\r\n");
}
this.Write(" </ItemGroup>\r\n</Project>\r\n");
returnthis.GenerationEnvironment.ToString();
}
It should be:
this.Write(this.ToStringHelper.ToStringWithCulture(Namespace));this.Write(".Model.csproj\" />\r\n </ItemGroup>\r\n\r\n <ItemGroup>\r\n");if(Target==CSharp.GenerateTarget.Cli){this.Write(" <ProjectCapability Include=\"MLNETCLIGenerated\" />\r\n");}else{this.Write(" <ProjectCapability Include=\"ModelBuilderGenerated\" />\r\n");}this.Write(" </ItemGroup>\r\n</Project>\r\n");returnthis.GenerationEnvironment.ToString();}
The generated .cs files are created from their corresponding .tt T4 template files. The change would occur there. I left a comment on the .cs as it shows the output.
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

@JakeRadMSFT it's auto-gen, I didn't modify those cs file a bit :)
JakeRadMSFT
JakeRadMSFT
reviewed
Apr 6, 2020
View reviewed changes
Comment threadsrc/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.tt
public bool IncludeRecommenderPackage {get;set;}
public string StablePackageVersion {get;set;}
public string UnstablePackageVersion {get;set;}
internal CSharp.GenerateTarget Target {get;set;}
@JakeRadMSFTJakeRadMSFTApr 6, 2020•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

NIT: I would think of CLI vs ModelBuilder as the Source instead of Target. It's the thing causing the projects to get generated.
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Will do
justinormont
justinormont
approved these changes
Apr 6, 2020
View reviewed changes
@justinormontjustinormont
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM; I'd recommend improving the code formatting: #5002 (review)
@justinormont
justinormont
commented
Apr 7, 2020•

edited

Loading

Copy link


Copy Markdown

Contributor
Current CI error on one Centos and one Ubuntu run is:
- ##[error]Unhandled exception. System.IO.IOException: No space left on device
@harishsk: Thoughts on how to resolve?
@harishsk
harishsk
commented
Apr 7, 2020

Copy link


Copy Markdown

Contributor
@harishsk: Thoughts on how to resolve?
@frank-dong-ms I believe you are already looking at this, aren't you?
@justinormont
justinormont
commented
Apr 7, 2020

Copy link


Copy Markdown

Contributor
Issues/PRs relating to running out of disk space on build machines:
@JakeRadMSFTJakeRadMSFT
changed the title
add projects capacities in CodeGeneratoradd projects capability in CodeGeneratorApr 7, 2020
@harishsk
harishsk
merged commit e451c80
into
dotnet:masterApr 7, 2020
@ghostghost
locked as resolved and limited conversation to collaborators
Mar 18, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@JakeRadMSFTJakeRadMSFTJakeRadMSFT approved these changes

+1 more reviewer

@justinormontjustinormontjustinormont approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@LittleLittleCloud@justinormont@harishsk@JakeRadMSFT

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
add projects capability in CodeGenerator - #5002
Merged
harishsk merged 1 commit into
dotnet:masterfrom 
LittleLittleCloud:u/xiaoyun/addProjectCapabilities
Apr 7, 2020
Merged
add projects capability in CodeGenerator#5002
harishsk merged 1 commit into
dotnet:masterfrom 
LittleLittleCloud:u/xiaoyun/addProjectCapabilities
Conversation
@LittleLittleCloud
@LittleLittleCloudLittleLittleCloud
commented
Apr 6, 2020

Copy link


Copy Markdown

Member
if generated by ModelBuilder, will add
<ProjectCapability Include="ModelBuilderGenerated" />

else
<ProjectCapability Include="MLNETCLIGenerated" />

to .Model and .Console project
@LittleLittleCloud
LittleLittleCloud
requested a review
from a team
as a code ownerApril 6, 2020 18:31
justinormont
justinormont
reviewed
Apr 6, 2020
View reviewed changes
Comment threadsrc/Microsoft.ML.CodeGenerator/Templates/Console/ModelProject.tt
 <ProjectCapability Include="MLNETCLIGenerated" />
<#}else{#>
 <ProjectCapability Include="ModelBuilderGenerated" />
<#}#>
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

What's diverging between CLI and MB that we need MLNETCLIGenerated vs ModelBuilderGenerated?
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

It's from our PM, we want to differentiate ModelBuilder-Generated from MLNet CLI Generated in telemetry data
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Sounds good.
justinormont
justinormont
reviewed
Apr 6, 2020
View reviewed changes
Comment threadsrc/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.cs
 this.Write(" <ProjectCapability Include=\"MLNETCLIGenerated\" />\r\n");
}else{
 this.Write(" <ProjectCapability Include=\"ModelBuilderGenerated\" />\r\n");
}
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Should modify the upstream .tt so the .cs is formatted properly here.
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Same w/ ModelProject.tt
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

What you mean
@JakeRadMSFTJakeRadMSFTApr 6, 2020

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

@LittleLittleCloud aren't these files generated from TT files?
@JakeRadMSFTJakeRadMSFTApr 6, 2020

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

In this PR it looks like the CS files were modified directly instead of changing the TT files.
@JakeRadMSFTJakeRadMSFTApr 6, 2020

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Nevermind I see them now :)
@justinormontjustinormontApr 6, 2020•

edited

Loading

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

The newly added code is currently being formatted as:
machinelearning/src/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.cs

Lines 77 to 86
in
e84006b
this.Write(this.ToStringHelper.ToStringWithCulture(Namespace));
this.Write(".Model.csproj\" />\r\n </ItemGroup>\r\n\r\n <ItemGroup>\r\n");
if(Target==CSharp.GenerateTarget.Cli){
this.Write(" <ProjectCapability Include=\"MLNETCLIGenerated\" />\r\n");
}else{
this.Write(" <ProjectCapability Include=\"ModelBuilderGenerated\" />\r\n");
}
this.Write(" </ItemGroup>\r\n</Project>\r\n");
returnthis.GenerationEnvironment.ToString();
}
It should be:
this.Write(this.ToStringHelper.ToStringWithCulture(Namespace));this.Write(".Model.csproj\" />\r\n </ItemGroup>\r\n\r\n <ItemGroup>\r\n");if(Target==CSharp.GenerateTarget.Cli){this.Write(" <ProjectCapability Include=\"MLNETCLIGenerated\" />\r\n");}else{this.Write(" <ProjectCapability Include=\"ModelBuilderGenerated\" />\r\n");}this.Write(" </ItemGroup>\r\n</Project>\r\n");returnthis.GenerationEnvironment.ToString();}
The generated .cs files are created from their corresponding .tt T4 template files. The change would occur there. I left a comment on the .cs as it shows the output.
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

@JakeRadMSFT it's auto-gen, I didn't modify those cs file a bit :)
JakeRadMSFT
JakeRadMSFT
reviewed
Apr 6, 2020
View reviewed changes
Comment threadsrc/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.tt
public bool IncludeRecommenderPackage {get;set;}
public string StablePackageVersion {get;set;}
public string UnstablePackageVersion {get;set;}
internal CSharp.GenerateTarget Target {get;set;}
@JakeRadMSFTJakeRadMSFTApr 6, 2020•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

NIT: I would think of CLI vs ModelBuilder as the Source instead of Target. It's the thing causing the projects to get generated.
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Will do
justinormont
justinormont
approved these changes
Apr 6, 2020
View reviewed changes
@justinormontjustinormont
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM; I'd recommend improving the code formatting: #5002 (review)
@justinormont
justinormont
commented
Apr 7, 2020•

edited

Loading

Copy link


Copy Markdown

Contributor
Current CI error on one Centos and one Ubuntu run is:
- ##[error]Unhandled exception. System.IO.IOException: No space left on device
@harishsk: Thoughts on how to resolve?
@harishsk
harishsk
commented
Apr 7, 2020

Copy link


Copy Markdown

Contributor
@harishsk: Thoughts on how to resolve?
@frank-dong-ms I believe you are already looking at this, aren't you?
@justinormont
justinormont
commented
Apr 7, 2020

Copy link


Copy Markdown

Contributor
Issues/PRs relating to running out of disk space on build machines:
@JakeRadMSFTJakeRadMSFT
changed the title
add projects capacities in CodeGeneratoradd projects capability in CodeGeneratorApr 7, 2020
@harishsk
harishsk
merged commit e451c80
into
dotnet:masterApr 7, 2020
@ghostghost
locked as resolved and limited conversation to collaborators
Mar 18, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@JakeRadMSFTJakeRadMSFTJakeRadMSFT approved these changes

+1 more reviewer

@justinormontjustinormontjustinormont approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@LittleLittleCloud@justinormont@harishsk@JakeRadMSFT

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
add projects capability in CodeGenerator - #5002
Merged
harishsk merged 1 commit into
dotnet:masterfrom 
LittleLittleCloud:u/xiaoyun/addProjectCapabilities
Apr 7, 2020
Merged
add projects capability in CodeGenerator#5002
harishsk merged 1 commit into
dotnet:masterfrom 
LittleLittleCloud:u/xiaoyun/addProjectCapabilities
Conversation
@LittleLittleCloud
@LittleLittleCloudLittleLittleCloud
commented
Apr 6, 2020

Copy link


Copy Markdown

Member
if generated by ModelBuilder, will add
<ProjectCapability Include="ModelBuilderGenerated" />

else
<ProjectCapability Include="MLNETCLIGenerated" />

to .Model and .Console project
@LittleLittleCloud
LittleLittleCloud
requested a review
from a team
as a code ownerApril 6, 2020 18:31
justinormont
justinormont
reviewed
Apr 6, 2020
View reviewed changes
Comment threadsrc/Microsoft.ML.CodeGenerator/Templates/Console/ModelProject.tt
 <ProjectCapability Include="MLNETCLIGenerated" />
<#}else{#>
 <ProjectCapability Include="ModelBuilderGenerated" />
<#}#>
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

What's diverging between CLI and MB that we need MLNETCLIGenerated vs ModelBuilderGenerated?
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

It's from our PM, we want to differentiate ModelBuilder-Generated from MLNet CLI Generated in telemetry data
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Sounds good.
justinormont
justinormont
reviewed
Apr 6, 2020
View reviewed changes
Comment threadsrc/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.cs
 this.Write(" <ProjectCapability Include=\"MLNETCLIGenerated\" />\r\n");
}else{
 this.Write(" <ProjectCapability Include=\"ModelBuilderGenerated\" />\r\n");
}
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Should modify the upstream .tt so the .cs is formatted properly here.
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Same w/ ModelProject.tt
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

What you mean
@JakeRadMSFTJakeRadMSFTApr 6, 2020

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

@LittleLittleCloud aren't these files generated from TT files?
@JakeRadMSFTJakeRadMSFTApr 6, 2020

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

In this PR it looks like the CS files were modified directly instead of changing the TT files.
@JakeRadMSFTJakeRadMSFTApr 6, 2020

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Nevermind I see them now :)
@justinormontjustinormontApr 6, 2020•

edited

Loading

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

The newly added code is currently being formatted as:
machinelearning/src/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.cs

Lines 77 to 86
in
e84006b
this.Write(this.ToStringHelper.ToStringWithCulture(Namespace));
this.Write(".Model.csproj\" />\r\n </ItemGroup>\r\n\r\n <ItemGroup>\r\n");
if(Target==CSharp.GenerateTarget.Cli){
this.Write(" <ProjectCapability Include=\"MLNETCLIGenerated\" />\r\n");
}else{
this.Write(" <ProjectCapability Include=\"ModelBuilderGenerated\" />\r\n");
}
this.Write(" </ItemGroup>\r\n</Project>\r\n");
returnthis.GenerationEnvironment.ToString();
}
It should be:
this.Write(this.ToStringHelper.ToStringWithCulture(Namespace));this.Write(".Model.csproj\" />\r\n </ItemGroup>\r\n\r\n <ItemGroup>\r\n");if(Target==CSharp.GenerateTarget.Cli){this.Write(" <ProjectCapability Include=\"MLNETCLIGenerated\" />\r\n");}else{this.Write(" <ProjectCapability Include=\"ModelBuilderGenerated\" />\r\n");}this.Write(" </ItemGroup>\r\n</Project>\r\n");returnthis.GenerationEnvironment.ToString();}
The generated .cs files are created from their corresponding .tt T4 template files. The change would occur there. I left a comment on the .cs as it shows the output.
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

@JakeRadMSFT it's auto-gen, I didn't modify those cs file a bit :)
JakeRadMSFT
JakeRadMSFT
reviewed
Apr 6, 2020
View reviewed changes
Comment threadsrc/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.tt
public bool IncludeRecommenderPackage {get;set;}
public string StablePackageVersion {get;set;}
public string UnstablePackageVersion {get;set;}
internal CSharp.GenerateTarget Target {get;set;}
@JakeRadMSFTJakeRadMSFTApr 6, 2020•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

NIT: I would think of CLI vs ModelBuilder as the Source instead of Target. It's the thing causing the projects to get generated.
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Will do
justinormont
justinormont
approved these changes
Apr 6, 2020
View reviewed changes
@justinormontjustinormont
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM; I'd recommend improving the code formatting: #5002 (review)
@justinormont
justinormont
commented
Apr 7, 2020•

edited

Loading

Copy link


Copy Markdown

Contributor
Current CI error on one Centos and one Ubuntu run is:
- ##[error]Unhandled exception. System.IO.IOException: No space left on device
@harishsk: Thoughts on how to resolve?
@harishsk
harishsk
commented
Apr 7, 2020

Copy link


Copy Markdown

Contributor
@harishsk: Thoughts on how to resolve?
@frank-dong-ms I believe you are already looking at this, aren't you?
@justinormont
justinormont
commented
Apr 7, 2020

Copy link


Copy Markdown

Contributor
Issues/PRs relating to running out of disk space on build machines:
@JakeRadMSFTJakeRadMSFT
changed the title
add projects capacities in CodeGeneratoradd projects capability in CodeGeneratorApr 7, 2020
@harishsk
harishsk
merged commit e451c80
into
dotnet:masterApr 7, 2020
@ghostghost
locked as resolved and limited conversation to collaborators
Mar 18, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@JakeRadMSFTJakeRadMSFTJakeRadMSFT approved these changes

+1 more reviewer

@justinormontjustinormontjustinormont approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@LittleLittleCloud@justinormont@harishsk@JakeRadMSFT

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
add projects capability in CodeGenerator - #5002
Merged
harishsk merged 1 commit into
dotnet:masterfrom 
LittleLittleCloud:u/xiaoyun/addProjectCapabilities
Apr 7, 2020
Merged
add projects capability in CodeGenerator#5002
harishsk merged 1 commit into
dotnet:masterfrom 
LittleLittleCloud:u/xiaoyun/addProjectCapabilities
Conversation
@LittleLittleCloud
@LittleLittleCloudLittleLittleCloud
commented
Apr 6, 2020

Copy link


Copy Markdown

Member
if generated by ModelBuilder, will add
<ProjectCapability Include="ModelBuilderGenerated" />

else
<ProjectCapability Include="MLNETCLIGenerated" />

to .Model and .Console project
@LittleLittleCloud
LittleLittleCloud
requested a review
from a team
as a code ownerApril 6, 2020 18:31
justinormont
justinormont
reviewed
Apr 6, 2020
View reviewed changes
Comment threadsrc/Microsoft.ML.CodeGenerator/Templates/Console/ModelProject.tt
 <ProjectCapability Include="MLNETCLIGenerated" />
<#}else{#>
 <ProjectCapability Include="ModelBuilderGenerated" />
<#}#>
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

What's diverging between CLI and MB that we need MLNETCLIGenerated vs ModelBuilderGenerated?
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

It's from our PM, we want to differentiate ModelBuilder-Generated from MLNet CLI Generated in telemetry data
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Sounds good.
justinormont
justinormont
reviewed
Apr 6, 2020
View reviewed changes
Comment threadsrc/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.cs
 this.Write(" <ProjectCapability Include=\"MLNETCLIGenerated\" />\r\n");
}else{
 this.Write(" <ProjectCapability Include=\"ModelBuilderGenerated\" />\r\n");
}
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Should modify the upstream .tt so the .cs is formatted properly here.
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Same w/ ModelProject.tt
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

What you mean
@JakeRadMSFTJakeRadMSFTApr 6, 2020

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

@LittleLittleCloud aren't these files generated from TT files?
@JakeRadMSFTJakeRadMSFTApr 6, 2020

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

In this PR it looks like the CS files were modified directly instead of changing the TT files.
@JakeRadMSFTJakeRadMSFTApr 6, 2020

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Nevermind I see them now :)
@justinormontjustinormontApr 6, 2020•

edited

Loading

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

The newly added code is currently being formatted as:
machinelearning/src/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.cs

Lines 77 to 86
in
e84006b
this.Write(this.ToStringHelper.ToStringWithCulture(Namespace));
this.Write(".Model.csproj\" />\r\n </ItemGroup>\r\n\r\n <ItemGroup>\r\n");
if(Target==CSharp.GenerateTarget.Cli){
this.Write(" <ProjectCapability Include=\"MLNETCLIGenerated\" />\r\n");
}else{
this.Write(" <ProjectCapability Include=\"ModelBuilderGenerated\" />\r\n");
}
this.Write(" </ItemGroup>\r\n</Project>\r\n");
returnthis.GenerationEnvironment.ToString();
}
It should be:
this.Write(this.ToStringHelper.ToStringWithCulture(Namespace));this.Write(".Model.csproj\" />\r\n </ItemGroup>\r\n\r\n <ItemGroup>\r\n");if(Target==CSharp.GenerateTarget.Cli){this.Write(" <ProjectCapability Include=\"MLNETCLIGenerated\" />\r\n");}else{this.Write(" <ProjectCapability Include=\"ModelBuilderGenerated\" />\r\n");}this.Write(" </ItemGroup>\r\n</Project>\r\n");returnthis.GenerationEnvironment.ToString();}
The generated .cs files are created from their corresponding .tt T4 template files. The change would occur there. I left a comment on the .cs as it shows the output.
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

@JakeRadMSFT it's auto-gen, I didn't modify those cs file a bit :)
JakeRadMSFT
JakeRadMSFT
reviewed
Apr 6, 2020
View reviewed changes
Comment threadsrc/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.tt
public bool IncludeRecommenderPackage {get;set;}
public string StablePackageVersion {get;set;}
public string UnstablePackageVersion {get;set;}
internal CSharp.GenerateTarget Target {get;set;}
@JakeRadMSFTJakeRadMSFTApr 6, 2020•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

NIT: I would think of CLI vs ModelBuilder as the Source instead of Target. It's the thing causing the projects to get generated.
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Will do
justinormont
justinormont
approved these changes
Apr 6, 2020
View reviewed changes
@justinormontjustinormont
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM; I'd recommend improving the code formatting: #5002 (review)
@justinormont
justinormont
commented
Apr 7, 2020•

edited

Loading

Copy link


Copy Markdown

Contributor
Current CI error on one Centos and one Ubuntu run is:
- ##[error]Unhandled exception. System.IO.IOException: No space left on device
@harishsk: Thoughts on how to resolve?
@harishsk
harishsk
commented
Apr 7, 2020

Copy link


Copy Markdown

Contributor
@harishsk: Thoughts on how to resolve?
@frank-dong-ms I believe you are already looking at this, aren't you?
@justinormont
justinormont
commented
Apr 7, 2020

Copy link


Copy Markdown

Contributor
Issues/PRs relating to running out of disk space on build machines:
@JakeRadMSFTJakeRadMSFT
changed the title
add projects capacities in CodeGeneratoradd projects capability in CodeGeneratorApr 7, 2020
@harishsk
harishsk
merged commit e451c80
into
dotnet:masterApr 7, 2020
@ghostghost
locked as resolved and limited conversation to collaborators
Mar 18, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@JakeRadMSFTJakeRadMSFTJakeRadMSFT approved these changes

+1 more reviewer

@justinormontjustinormontjustinormont approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@LittleLittleCloud@justinormont@harishsk@JakeRadMSFT

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
add projects capability in CodeGenerator - #5002
Merged
harishsk merged 1 commit into
dotnet:masterfrom 
LittleLittleCloud:u/xiaoyun/addProjectCapabilities
Apr 7, 2020
Merged
add projects capability in CodeGenerator#5002
harishsk merged 1 commit into
dotnet:masterfrom 
LittleLittleCloud:u/xiaoyun/addProjectCapabilities
Conversation
@LittleLittleCloud
@LittleLittleCloudLittleLittleCloud
commented
Apr 6, 2020

Copy link


Copy Markdown

Member
if generated by ModelBuilder, will add
<ProjectCapability Include="ModelBuilderGenerated" />

else
<ProjectCapability Include="MLNETCLIGenerated" />

to .Model and .Console project
@LittleLittleCloud
LittleLittleCloud
requested a review
from a team
as a code ownerApril 6, 2020 18:31
justinormont
justinormont
reviewed
Apr 6, 2020
View reviewed changes
Comment threadsrc/Microsoft.ML.CodeGenerator/Templates/Console/ModelProject.tt
 <ProjectCapability Include="MLNETCLIGenerated" />
<#}else{#>
 <ProjectCapability Include="ModelBuilderGenerated" />
<#}#>
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

What's diverging between CLI and MB that we need MLNETCLIGenerated vs ModelBuilderGenerated?
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

It's from our PM, we want to differentiate ModelBuilder-Generated from MLNet CLI Generated in telemetry data
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Sounds good.
justinormont
justinormont
reviewed
Apr 6, 2020
View reviewed changes
Comment threadsrc/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.cs
 this.Write(" <ProjectCapability Include=\"MLNETCLIGenerated\" />\r\n");
}else{
 this.Write(" <ProjectCapability Include=\"ModelBuilderGenerated\" />\r\n");
}
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Should modify the upstream .tt so the .cs is formatted properly here.
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Same w/ ModelProject.tt
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

What you mean
@JakeRadMSFTJakeRadMSFTApr 6, 2020

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

@LittleLittleCloud aren't these files generated from TT files?
@JakeRadMSFTJakeRadMSFTApr 6, 2020

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

In this PR it looks like the CS files were modified directly instead of changing the TT files.
@JakeRadMSFTJakeRadMSFTApr 6, 2020

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Nevermind I see them now :)
@justinormontjustinormontApr 6, 2020•

edited

Loading

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

The newly added code is currently being formatted as:
machinelearning/src/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.cs

Lines 77 to 86
in
e84006b
this.Write(this.ToStringHelper.ToStringWithCulture(Namespace));
this.Write(".Model.csproj\" />\r\n </ItemGroup>\r\n\r\n <ItemGroup>\r\n");
if(Target==CSharp.GenerateTarget.Cli){
this.Write(" <ProjectCapability Include=\"MLNETCLIGenerated\" />\r\n");
}else{
this.Write(" <ProjectCapability Include=\"ModelBuilderGenerated\" />\r\n");
}
this.Write(" </ItemGroup>\r\n</Project>\r\n");
returnthis.GenerationEnvironment.ToString();
}
It should be:
this.Write(this.ToStringHelper.ToStringWithCulture(Namespace));this.Write(".Model.csproj\" />\r\n </ItemGroup>\r\n\r\n <ItemGroup>\r\n");if(Target==CSharp.GenerateTarget.Cli){this.Write(" <ProjectCapability Include=\"MLNETCLIGenerated\" />\r\n");}else{this.Write(" <ProjectCapability Include=\"ModelBuilderGenerated\" />\r\n");}this.Write(" </ItemGroup>\r\n</Project>\r\n");returnthis.GenerationEnvironment.ToString();}
The generated .cs files are created from their corresponding .tt T4 template files. The change would occur there. I left a comment on the .cs as it shows the output.
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

@JakeRadMSFT it's auto-gen, I didn't modify those cs file a bit :)
JakeRadMSFT
JakeRadMSFT
reviewed
Apr 6, 2020
View reviewed changes
Comment threadsrc/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.tt
public bool IncludeRecommenderPackage {get;set;}
public string StablePackageVersion {get;set;}
public string UnstablePackageVersion {get;set;}
internal CSharp.GenerateTarget Target {get;set;}
@JakeRadMSFTJakeRadMSFTApr 6, 2020•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

NIT: I would think of CLI vs ModelBuilder as the Source instead of Target. It's the thing causing the projects to get generated.
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Will do
justinormont
justinormont
approved these changes
Apr 6, 2020
View reviewed changes
@justinormontjustinormont
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM; I'd recommend improving the code formatting: #5002 (review)
@justinormont
justinormont
commented
Apr 7, 2020•

edited

Loading

Copy link


Copy Markdown

Contributor
Current CI error on one Centos and one Ubuntu run is:
- ##[error]Unhandled exception. System.IO.IOException: No space left on device
@harishsk: Thoughts on how to resolve?
@harishsk
harishsk
commented
Apr 7, 2020

Copy link


Copy Markdown

Contributor
@harishsk: Thoughts on how to resolve?
@frank-dong-ms I believe you are already looking at this, aren't you?
@justinormont
justinormont
commented
Apr 7, 2020

Copy link


Copy Markdown

Contributor
Issues/PRs relating to running out of disk space on build machines:
@JakeRadMSFTJakeRadMSFT
changed the title
add projects capacities in CodeGeneratoradd projects capability in CodeGeneratorApr 7, 2020
@harishsk
harishsk
merged commit e451c80
into
dotnet:masterApr 7, 2020
@ghostghost
locked as resolved and limited conversation to collaborators
Mar 18, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@JakeRadMSFTJakeRadMSFTJakeRadMSFT approved these changes

+1 more reviewer

@justinormontjustinormontjustinormont approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@LittleLittleCloud@justinormont@harishsk@JakeRadMSFT

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
add projects capability in CodeGenerator - #5002
Merged
harishsk merged 1 commit into
dotnet:masterfrom 
LittleLittleCloud:u/xiaoyun/addProjectCapabilities
Apr 7, 2020
Merged
add projects capability in CodeGenerator#5002
harishsk merged 1 commit into
dotnet:masterfrom 
LittleLittleCloud:u/xiaoyun/addProjectCapabilities
Conversation
@LittleLittleCloud
@LittleLittleCloudLittleLittleCloud
commented
Apr 6, 2020

Copy link


Copy Markdown

Member
if generated by ModelBuilder, will add
<ProjectCapability Include="ModelBuilderGenerated" />

else
<ProjectCapability Include="MLNETCLIGenerated" />

to .Model and .Console project
@LittleLittleCloud
LittleLittleCloud
requested a review
from a team
as a code ownerApril 6, 2020 18:31
justinormont
justinormont
reviewed
Apr 6, 2020
View reviewed changes
Comment threadsrc/Microsoft.ML.CodeGenerator/Templates/Console/ModelProject.tt
 <ProjectCapability Include="MLNETCLIGenerated" />
<#}else{#>
 <ProjectCapability Include="ModelBuilderGenerated" />
<#}#>
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

What's diverging between CLI and MB that we need MLNETCLIGenerated vs ModelBuilderGenerated?
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

It's from our PM, we want to differentiate ModelBuilder-Generated from MLNet CLI Generated in telemetry data
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Sounds good.
justinormont
justinormont
reviewed
Apr 6, 2020
View reviewed changes
Comment threadsrc/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.cs
 this.Write(" <ProjectCapability Include=\"MLNETCLIGenerated\" />\r\n");
}else{
 this.Write(" <ProjectCapability Include=\"ModelBuilderGenerated\" />\r\n");
}
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Should modify the upstream .tt so the .cs is formatted properly here.
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Same w/ ModelProject.tt
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

What you mean
@JakeRadMSFTJakeRadMSFTApr 6, 2020

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

@LittleLittleCloud aren't these files generated from TT files?
@JakeRadMSFTJakeRadMSFTApr 6, 2020

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

In this PR it looks like the CS files were modified directly instead of changing the TT files.
@JakeRadMSFTJakeRadMSFTApr 6, 2020

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Nevermind I see them now :)
@justinormontjustinormontApr 6, 2020•

edited

Loading

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

The newly added code is currently being formatted as:
machinelearning/src/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.cs

Lines 77 to 86
in
e84006b
this.Write(this.ToStringHelper.ToStringWithCulture(Namespace));
this.Write(".Model.csproj\" />\r\n </ItemGroup>\r\n\r\n <ItemGroup>\r\n");
if(Target==CSharp.GenerateTarget.Cli){
this.Write(" <ProjectCapability Include=\"MLNETCLIGenerated\" />\r\n");
}else{
this.Write(" <ProjectCapability Include=\"ModelBuilderGenerated\" />\r\n");
}
this.Write(" </ItemGroup>\r\n</Project>\r\n");
returnthis.GenerationEnvironment.ToString();
}
It should be:
this.Write(this.ToStringHelper.ToStringWithCulture(Namespace));this.Write(".Model.csproj\" />\r\n </ItemGroup>\r\n\r\n <ItemGroup>\r\n");if(Target==CSharp.GenerateTarget.Cli){this.Write(" <ProjectCapability Include=\"MLNETCLIGenerated\" />\r\n");}else{this.Write(" <ProjectCapability Include=\"ModelBuilderGenerated\" />\r\n");}this.Write(" </ItemGroup>\r\n</Project>\r\n");returnthis.GenerationEnvironment.ToString();}
The generated .cs files are created from their corresponding .tt T4 template files. The change would occur there. I left a comment on the .cs as it shows the output.
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

@JakeRadMSFT it's auto-gen, I didn't modify those cs file a bit :)
JakeRadMSFT
JakeRadMSFT
reviewed
Apr 6, 2020
View reviewed changes
Comment threadsrc/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.tt
public bool IncludeRecommenderPackage {get;set;}
public string StablePackageVersion {get;set;}
public string UnstablePackageVersion {get;set;}
internal CSharp.GenerateTarget Target {get;set;}
@JakeRadMSFTJakeRadMSFTApr 6, 2020•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

NIT: I would think of CLI vs ModelBuilder as the Source instead of Target. It's the thing causing the projects to get generated.
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Will do
justinormont
justinormont
approved these changes
Apr 6, 2020
View reviewed changes
@justinormontjustinormont
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM; I'd recommend improving the code formatting: #5002 (review)
@justinormont
justinormont
commented
Apr 7, 2020•

edited

Loading

Copy link


Copy Markdown

Contributor
Current CI error on one Centos and one Ubuntu run is:
- ##[error]Unhandled exception. System.IO.IOException: No space left on device
@harishsk: Thoughts on how to resolve?
@harishsk
harishsk
commented
Apr 7, 2020

Copy link


Copy Markdown

Contributor
@harishsk: Thoughts on how to resolve?
@frank-dong-ms I believe you are already looking at this, aren't you?
@justinormont
justinormont
commented
Apr 7, 2020

Copy link


Copy Markdown

Contributor
Issues/PRs relating to running out of disk space on build machines:
@JakeRadMSFTJakeRadMSFT
changed the title
add projects capacities in CodeGeneratoradd projects capability in CodeGeneratorApr 7, 2020
@harishsk
harishsk
merged commit e451c80
into
dotnet:masterApr 7, 2020
@ghostghost
locked as resolved and limited conversation to collaborators
Mar 18, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@JakeRadMSFTJakeRadMSFTJakeRadMSFT approved these changes

+1 more reviewer

@justinormontjustinormontjustinormont approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@LittleLittleCloud@justinormont@harishsk@JakeRadMSFT

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
add projects capability in CodeGenerator - #5002
Merged
harishsk merged 1 commit into
dotnet:masterfrom 
LittleLittleCloud:u/xiaoyun/addProjectCapabilities
Apr 7, 2020
Merged
add projects capability in CodeGenerator#5002
harishsk merged 1 commit into
dotnet:masterfrom 
LittleLittleCloud:u/xiaoyun/addProjectCapabilities
Conversation
@LittleLittleCloud
@LittleLittleCloudLittleLittleCloud
commented
Apr 6, 2020

Copy link


Copy Markdown

Member
if generated by ModelBuilder, will add
<ProjectCapability Include="ModelBuilderGenerated" />

else
<ProjectCapability Include="MLNETCLIGenerated" />

to .Model and .Console project
@LittleLittleCloud
LittleLittleCloud
requested a review
from a team
as a code ownerApril 6, 2020 18:31
justinormont
justinormont
reviewed
Apr 6, 2020
View reviewed changes
Comment threadsrc/Microsoft.ML.CodeGenerator/Templates/Console/ModelProject.tt
 <ProjectCapability Include="MLNETCLIGenerated" />
<#}else{#>
 <ProjectCapability Include="ModelBuilderGenerated" />
<#}#>
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

What's diverging between CLI and MB that we need MLNETCLIGenerated vs ModelBuilderGenerated?
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

It's from our PM, we want to differentiate ModelBuilder-Generated from MLNet CLI Generated in telemetry data
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Sounds good.
justinormont
justinormont
reviewed
Apr 6, 2020
View reviewed changes
Comment threadsrc/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.cs
 this.Write(" <ProjectCapability Include=\"MLNETCLIGenerated\" />\r\n");
}else{
 this.Write(" <ProjectCapability Include=\"ModelBuilderGenerated\" />\r\n");
}
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Should modify the upstream .tt so the .cs is formatted properly here.
@justinormontjustinormontApr 6, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Same w/ ModelProject.tt
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

What you mean
@JakeRadMSFTJakeRadMSFTApr 6, 2020

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

@LittleLittleCloud aren't these files generated from TT files?
@JakeRadMSFTJakeRadMSFTApr 6, 2020

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

In this PR it looks like the CS files were modified directly instead of changing the TT files.
@JakeRadMSFTJakeRadMSFTApr 6, 2020

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Nevermind I see them now :)
@justinormontjustinormontApr 6, 2020•

edited

Loading

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

The newly added code is currently being formatted as:
machinelearning/src/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.cs

Lines 77 to 86
in
e84006b
this.Write(this.ToStringHelper.ToStringWithCulture(Namespace));
this.Write(".Model.csproj\" />\r\n </ItemGroup>\r\n\r\n <ItemGroup>\r\n");
if(Target==CSharp.GenerateTarget.Cli){
this.Write(" <ProjectCapability Include=\"MLNETCLIGenerated\" />\r\n");
}else{
this.Write(" <ProjectCapability Include=\"ModelBuilderGenerated\" />\r\n");
}
this.Write(" </ItemGroup>\r\n</Project>\r\n");
returnthis.GenerationEnvironment.ToString();
}
It should be:
this.Write(this.ToStringHelper.ToStringWithCulture(Namespace));this.Write(".Model.csproj\" />\r\n </ItemGroup>\r\n\r\n <ItemGroup>\r\n");if(Target==CSharp.GenerateTarget.Cli){this.Write(" <ProjectCapability Include=\"MLNETCLIGenerated\" />\r\n");}else{this.Write(" <ProjectCapability Include=\"ModelBuilderGenerated\" />\r\n");}this.Write(" </ItemGroup>\r\n</Project>\r\n");returnthis.GenerationEnvironment.ToString();}
The generated .cs files are created from their corresponding .tt T4 template files. The change would occur there. I left a comment on the .cs as it shows the output.
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

@JakeRadMSFT it's auto-gen, I didn't modify those cs file a bit :)
JakeRadMSFT
JakeRadMSFT
reviewed
Apr 6, 2020
View reviewed changes
Comment threadsrc/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.tt
public bool IncludeRecommenderPackage {get;set;}
public string StablePackageVersion {get;set;}
public string UnstablePackageVersion {get;set;}
internal CSharp.GenerateTarget Target {get;set;}
@JakeRadMSFTJakeRadMSFTApr 6, 2020•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

NIT: I would think of CLI vs ModelBuilder as the Source instead of Target. It's the thing causing the projects to get generated.
@LittleLittleCloudLittleLittleCloudApr 6, 2020

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Will do
justinormont
justinormont
approved these changes
Apr 6, 2020
View reviewed changes
@justinormontjustinormont
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM; I'd recommend improving the code formatting: #5002 (review)
@justinormont
justinormont
commented
Apr 7, 2020•

edited

Loading

Copy link


Copy Markdown

Contributor
Current CI error on one Centos and one Ubuntu run is:
- ##[error]Unhandled exception. System.IO.IOException: No space left on device
@harishsk: Thoughts on how to resolve?
@harishsk
harishsk
commented
Apr 7, 2020

Copy link


Copy Markdown

Contributor
@harishsk: Thoughts on how to resolve?
@frank-dong-ms I believe you are already looking at this, aren't you?
@justinormont
justinormont
commented
Apr 7, 2020

Copy link


Copy Markdown

Contributor
Issues/PRs relating to running out of disk space on build machines:
@JakeRadMSFTJakeRadMSFT
changed the title
add projects capacities in CodeGeneratoradd projects capability in CodeGeneratorApr 7, 2020
@harishsk
harishsk
merged commit e451c80
into
dotnet:masterApr 7, 2020
@ghostghost
locked as resolved and limited conversation to collaborators
Mar 18, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@JakeRadMSFTJakeRadMSFTJakeRadMSFT approved these changes

+1 more reviewer

@justinormontjustinormontjustinormont approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@LittleLittleCloud@justinormont@harishsk@JakeRadMSFT