Skip to content

Add F# samples - #36

Merged
eerhardt merged 5 commits into
dotnet:masterfrom
dsyme:fs3b
Aug 7, 2018
Merged

eerhardt merged 5 commits into
dotnet:masterfrom
dsyme:fs3b

Conversation

@dsyme

@dsyme dsyme commented Aug 3, 2018 •

Copy link
Copy Markdown
Contributor

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

Things to do:

@CESARDELATORRE

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

dsyme commented Aug 3, 2018 •

Copy link
Copy Markdown
Contributor Author

@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

ankitbko commented Aug 3, 2018

Copy link
Copy Markdown
Member

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

@dsyme

dsyme commented Aug 3, 2018

Copy link
Copy Markdown
Contributor Author

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

Comment thread samples/nuget.config Outdated
<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
Copy Markdown
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