fsprojects / IfSharp

F# for Jupyter Notebooks
Other
440 stars 71 forks source link

Adding dependencies from GitHub, Git #179

Closed lucymukh closed 5 years ago

lucymukh commented 6 years ago

The code in pull request allows to conveniently add dependencies from Git and GitHub. Paket .Net API currently doesn't support HTTP dependencies yet.

GitHub dependency usage example: Paket.GitHub ["user/repo:version file"] , where version and file are optional Sample: http://nbviewer.ipython.org/github/lucymukh/IfSharp/blob/ad04dc50560c8bdd45263330b38ac88437ce957e/docs/files/notebooks/GitHub%20dependencies%20sample.ipynb

Git dependency usage example: Paket.Git ["repo version"] , where version is optional

Maybe the functionality sample is worth adding to the FSharp Jupyter Notebook?

cgravill commented 6 years ago

This looks great. Sure, please do add to the sample documentation. As a small suggestion, instead of using:

#r "packages/Angara.Table/lib/net452/Angara.Table.dll"
#r "packages/Angara.Statistics/lib/net452/Angara.Statistics.dll"

it should be possible to

#load "Paket.Generated.Refs.fsx"

and avoid embedding the framework versions into the notebook - that causes issues when the NuGets themselves update.

cgravill commented 6 years ago

@evelinag would this syntax address work for you? https://github.com/fsprojects/IfSharp/issues/146#issuecomment-302447810

lucymukh commented 6 years ago

Thank you for the remarks, Colin. I've changed the sample accordingly and added it at the end of the FSharp_Jupyter_Notebooks notebook

cgravill commented 6 years ago

Great, looks good.

I saw you're having to do quite a lot of the work of breaking up the string with a regular expression before passing it to Paket. Presumably Paket has to do something similar itself. Is that functionality not available in the PaketCore API?

lucymukh commented 5 years ago

@cgravill the parsing code exists, but is not exposed as public. To avoid obscure RegExp validation I can propose the following combinator-based alternative:

let npzDep =
    Paket.gitHubRepo "predictionmachines/Angara.Chart"
    |> Paket.chooseBranch "gh-pages"
    |> Paket.singleFile "data/npz.csv"
let siteDep =
    Paket.gitHubRepo "predictionmachines/Angara.Chart"
    |> Paket.chooseBranch "gh-pages"
    |> Paket.singleFile "data/site.csv"
Paket.Git [npzDep; siteDep]

The pull request is updated

cgravill commented 5 years ago

Oh, I didn't mean to request changing just wanted to make sure we weren't duplicating functionality in Paket.

Maintaining the style and interoperability with https://fsprojects.github.io/Paket/github-dependencies.html seems like a good idea. If the regular expression approach turned out to have issues we could always request the Paket folks expose the functionality.

The combinator approach looks a lot more explicit and safe but people might find it a bit much in practice.

lucymukh commented 5 years ago

@cgravill, ok, I've reverted the last commit. Did I understand you correctly that my regExp check works for now?

cgravill commented 5 years ago

Yes, that's great for now. I'll merge.