Skip to content

F# samples #7

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

Closed
wants to merge 4 commits into from
Closed

Conversation

isaacabraham
Copy link

This is a simple port of the Sentiment Analysis sample to F#. This also puts in place the necessary plumbing for other people to easily add new sample scripts (and full-blown apps if required).

Happy to help you get up and running if needed, although there's a readme in the folder with some basic instructions.

@dnfclas
Copy link

dnfclas commented Jun 6, 2018

CLA assistant check
All CLA requirements met.

System.Environment.SetEnvironmentVariable("Path", System.Environment.GetEnvironmentVariable "Path" + ";" + nativeDirectory)

module Datasets =
let private buildPath file = System.IO.Path.Combine(@"..\datasets\", file)
Copy link

Choose a reason for hiding this comment

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

I'm just wondering why this path is pointing to just one folder up instead of two folders up like ..\..\datasets\. Currently I'm struggling a bit because compiler is looking for dataset in getting-started directory. But maybe I'm doing something wrong.

Copy link

@markpattison markpattison left a comment

Choose a reason for hiding this comment

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

I also had an issue with the path for the data, but in my case it was looking for dataset in the directory above the repository, so the fix was to use .\datasets\ rather than ..\datasets\. I assume this is nothing to do with the sample code but rather which directory is current when FSI is launched?

Otherwise, works great.

@isaacabraham
Copy link
Author

@robosek @markpattison Yes, I think this is the issue - where you open your IDE. For me, I used VS Code and opened the IDE in the fsharp-samples folder. Evidently, if you open in a different location, the paths don't quite work! Perhaps I'll just change it to assume you're in the root of the repo?

@robosek
Copy link

robosek commented Jun 8, 2018

Oh right @isaacabraham the solution is to open just fsharp-samples in VSCode. Thanks! In my opinion it's ok but maybe some short note about this in Readme file could be helpful to avoid confusion?

@isaacabraham
Copy link
Author

@robosek great idea.

@isaacabraham
Copy link
Author

@robosek @markpattison let me know if it's now any better.

Copy link

@robosek robosek left a comment

Choose a reason for hiding this comment

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

Great work! Thanks for make it easy to play ML.NET with FSI 👍

Copy link

@markpattison markpattison left a comment

Choose a reason for hiding this comment

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

Looks good.

@isaacabraham
Copy link
Author

@OliaG it would be great if you could give everyone interested in this an update on what's needed to get this PR accepted - thanks!

@OliaG
Copy link
Member

OliaG commented Jun 18, 2018

@isaacabraham thank you very much for your contribution! We'd like to make changes in ML.NET that would provide better support for F#. We are planing to do it in the next coming version v 0.3. So this PR would probably need some updates. After that we'll gladly add it to the repo.

@isaacabraham
Copy link
Author

@OliaG with 0.3 released, is the plumbing now in place for me to update this PR?

@OliaG
Copy link
Member

OliaG commented Jul 8, 2018

@isaacabraham we did a small reorg, now it will be @CESARDELATORRE or @asthana86 to answer this question.

@CESARDELATORRE
Copy link
Contributor

@isaacabraham @OliaG - Thanks a lot Isaac for your efforts and PR. You can test v0.3 with your code and see what's new in it in the following blog post we published here:
https://github.com/dotnet/machinelearning/releases/tag/v0.3.0

About merging the PR into the repo, it is pretty possible that we'll make significant breaking changes in the API in upcoming versions, so it might be better if we hold on this PR (and additional samples) just for a while.
Also, it makes sense to wait until we make announcements about "F# and ML.NET", and precisely we'll use this and other samples as new F# code examples for that push.

In any case, please tell us when you have tested the code with v0.3, ok?

Again, thanks a lot for your PR, it'll be a great help for the community. 👍

@isaacabraham
Copy link
Author

@CESARDELATORRE Reading through the release notes, I'm not sure that anything has changed since v0.2 i.e. is there support for records (or even class properties)? And is there better support for in-memory data?

If neither, then there's not much that can be updated since those are the two main blockers at the moment.

Cheers

@CESARDELATORRE
Copy link
Contributor

@isaacabraham - As far as I know, there are no specific features in v0.3 with improvements for F#.
I was not aware that you had blocking issues.
In any case, we need to wait until ML.NET is prepared for F#, from different angles as well, in addition to the specific blocking issues you mentioned.

@asthana86
Copy link
Contributor

@CESARDELATORRE , @isaacabraham Yep we are starting to work with @dsyme to get the story going for F# and ML.NET. Happy to include you on the conversations :).

@isaacabraham
Copy link
Author

@CESARDELATORRE I think @asthana86 is aware of them :-) Probably the biggest two blockers are dotnet/machinelearning#180 and dotnet/machinelearning#91

@isaacabraham
Copy link
Author

Closing in favour of #36

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants