Skip to content
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

Add F# samples #36

Merged
merged 5 commits into from
Aug 7, 2018
Merged

Add F# samples #36

merged 5 commits into from
Aug 7, 2018

Conversation

dsyme
Copy link
Contributor

@dsyme dsyme commented Aug 3, 2018

Variation on #32 to add F# samples after moving C# samples to "csharp" directory

Things to do:

@CESARDELATORRE
Copy link
Contributor

Would it be possible to add the F# samples (this PR) to the branch "dotnet/machinelearning-samples" instead of MASTER branch? - Until F# is supported in the public release of ML.NET.

I'll add the Csharp changes about the folders first, to MASTER and to "dotnet/machinelearning-samples" branches, though.

@CESARDELATORRE
Copy link
Contributor

Never mind, I just saw your email. Let's keep the PR against MASTER. But we'll need to wait for the merge to happen.

@dsyme
Copy link
Contributor Author

dsyme commented Aug 3, 2018

@asthana86 @CESARDELATORRE This change is now much smaller now #35 is in, if you'd like to start reading the F#. Main points to note are

  • Each Program.fs is written in top-to-bottom script form, e.g. like this.

  • We may eventually change them to be "script.fsx" files but for now they are compiled, for now that's fine

  • When reading the code, you start reading at the top and go through to the bottom just like a python script. Functions are defined before they get called.

  • I've just used a single file for each example. F# people tend to put more in a file since definitions are generally more succinct.

  • The data types are defined using member val. F# people would normally like to use records without any attributes and it still feels a little odd to use classes and attributes in F#. It would be nice to look at whether this can become simpler

  • I think the plotting and CSV data access can be simplified, to use XPlot and FSharp.Data respectively, I'll look into that.

Good intro guides to F# are at https://fsharpforfunandprofit.com/

@ankitbko
Copy link
Member

ankitbko commented Aug 3, 2018

@dsyme Pretty sure I am not the person you intended to tag 😄

@dsyme
Copy link
Contributor Author

dsyme commented Aug 3, 2018

@ankitbko - You're right, was after @asthana86 :)

@@ -5,5 +5,6 @@
<add key="nuget.org" value="https://api.nuget.org/v3/index.json" />
<!-- Use this if you want consume the daily builds of ML.NET -->
<!-- <add key="Dailies" value="https://dotnet.myget.org/F/dotnet-core/api/v3/index.json" /> -->
<add key="localPath" value="C:\GitHub\dsyme\machinelearning\bin/packages" />
Copy link
Member

Choose a reason for hiding this comment

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

This should be reverted. It is causing CI to fail.

@eerhardt eerhardt merged commit b8fed8b into dotnet:master Aug 7, 2018
@isaacabraham isaacabraham mentioned this pull request Aug 9, 2018
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.

4 participants