Skip to content

Rename Microsoft.ML to Microsoft.ML.Legacy - #920

Merged
Zruty0 merged 6 commits into
dotnet:masterfrom
Zruty0:feature/legacy-project
Sep 17, 2018
Merged

Rename Microsoft.ML to Microsoft.ML.Legacy#920
Zruty0 merged 6 commits into
dotnet:masterfrom
Zruty0:feature/legacy-project

Conversation

@Zruty0

@Zruty0 Zruty0 commented Sep 14, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

This is a purely mechanical rename.

  • Microsoft.ML -> Microsoft.ML.Legacy
  • Moved to src/Microsoft.ML.Legacy
  • Updated project references and solution

@Zruty0 Zruty0 added the api label Sep 14, 2018
@Zruty0 Zruty0 self-assigned this Sep 14, 2018
@eerhardt

eerhardt commented Sep 14, 2018 •
edited by Zruty0
Loading

Copy link
Copy Markdown
Member

Quick question: are we eventually going to delete all the entry point code (and code gen) in this project as well? Not just the learning pipeline API? #Resolved

@Zruty0

Zruty0 commented Sep 14, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

I would delete the codegen, but keep the entry points. #Resolved

eerhardt
eerhardt approved these changes Sep 14, 2018
View reviewed changes
Comment thread src/Microsoft.ML.HalLearners/Microsoft.ML.HalLearners.csproj Outdated
<ProjectReference Include="..\Microsoft.ML.Data\Microsoft.ML.Data.csproj" />
<ProjectReference Include="..\Microsoft.ML.StandardLearners\Microsoft.ML.StandardLearners.csproj" />
<ProjectReference Include="..\Microsoft.ML\Microsoft.ML.csproj" />
<ProjectReference Include="..\Microsoft.ML.Legacy\Microsoft.ML.Legacy.csproj" />

@eerhardt eerhardt Sep 14, 2018 •
edited by Zruty0
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.

Are these src references concerning? What are we going to do when we delete the legacy assembly? #Resolved

@Zruty0 Zruty0 Sep 14, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

They are indeed concerning.

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

@Zruty0 Zruty0 Sep 14, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I removed those I could from src. I wouldn't worry about test for now.
The remaining ones are PipelineInference and StandardLearners (there's a dependency for OVA entry point, which we'll have to fix, but not now)

In reply to: 217854702 [](ancestors = 217854702,217852426)

@eerhardt eerhardt Sep 14, 2018

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.

Sounds good to me. Maybe the OVA entry point can go next to the OVA API?

Comment thread test/Microsoft.ML.Benchmarks/Microsoft.ML.Benchmarks.csproj Outdated
<None Condition="Exists('..\data\external\WikiDetoxAnnotated160kRows.tsv')"
Include="..\data\external\WikiDetoxAnnotated160kRows.tsv"
Link="external\WikiDetoxAnnotated160kRows.tsv">
<None Condition="Exists('..\data\external\WikiDetoxAnnotated160kRows.tsv')" Include="..\data\external\WikiDetoxAnnotated160kRows.tsv" Link="external\WikiDetoxAnnotated160kRows.tsv">

@eerhardt eerhardt Sep 14, 2018 •
edited by Zruty0
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.

Did VS do this formatting? The existing format is easier to read. #Closed

@Zruty0 Zruty0 Sep 14, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep, VS. I reverted

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

@eerhardt eerhardt Sep 14, 2018 •
edited by Zruty0
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.

Well then I guess I wouldn’t worry about it because fighting VS is usually a losing battle... #Closed

sfilipi
sfilipi approved these changes Sep 15, 2018
View reviewed changes

@sfilipi sfilipi left a comment

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.

:shipit:

@Zruty0
Zruty0 merged commit a8b844c into dotnet:master Sep 17, 2018
@Zruty0
Zruty0 deleted the feature/legacy-project branch September 17, 2018 20:06
@TomFinley TomFinley mentioned this pull request Nov 4, 2018
Closed
@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.

Reviewers

@Ivanidzo4ka Ivanidzo4ka Awaiting requested review from Ivanidzo4ka
@TomFinley TomFinley Awaiting requested review from TomFinley
2 more reviewers
@eerhardt eerhardt eerhardt approved these changes
@sfilipi sfilipi sfilipi approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@Zruty0 Zruty0

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Zruty0 @eerhardt @sfilipi