Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Sep 17, 2018
Merged

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

merged 6 commits into from
Sep 17, 2018

Conversation

Zruty0
Copy link
Contributor

@Zruty0 Zruty0 commented Sep 14, 2018 •
edited
Loading

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 Issues pertaining the friendly API label Sep 14, 2018
@Zruty0 Zruty0 self-assigned this Sep 14, 2018
@eerhardt
Copy link
Member

eerhardt commented Sep 14, 2018 •
edited by Zruty0
Loading

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
Copy link
Contributor Author

Zruty0 commented Sep 14, 2018 •
edited
Loading

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

eerhardt
eerhardt approved these changes Sep 14, 2018
View reviewed changes
src/Microsoft.ML.HalLearners/Microsoft.ML.HalLearners.csproj Outdated
@@ -10,7 +10,7 @@
<ProjectReference Include="..\Microsoft.ML.Core\Microsoft.ML.Core.csproj" />
<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" />
Copy link
Member

@eerhardt eerhardt Sep 14, 2018 •
edited by Zruty0
Loading

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

Copy link
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)

Copy link
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)

Copy link
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?

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">
Copy link
Member

@eerhardt eerhardt Sep 14, 2018 •
edited by Zruty0
Loading

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

Copy link
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)

Copy link
Member

@eerhardt eerhardt Sep 14, 2018 •
edited by Zruty0
Loading

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
Copy link
Member

@sfilipi sfilipi left a comment

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

@eerhardt eerhardt eerhardt approved these changes

@sfilipi sfilipi sfilipi approved these changes

@Ivanidzo4ka Ivanidzo4ka Awaiting requested review from Ivanidzo4ka

@TomFinley TomFinley Awaiting requested review from TomFinley

Assignees

@Zruty0 Zruty0

Labels
API Issues pertaining the friendly API
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Zruty0 @eerhardt @sfilipi