fsprojects / IfSharp

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

AsyncSeq display & examples #189

Closed dgrechka closed 6 years ago

dgrechka commented 6 years ago

Contains AsyncSeq display code that pulls the elements out of asyncSeq and displays the most recent one.

The demo notebook is also updated with the AsyncSeq examples.

The async display code is refactored so all Async and AsyncSeq handling is now moved out of the kernel.fs

The code is tested both on Windows and Ubuntu. The demo notebook works.

Related to #104

dgrechka commented 6 years ago

Wait a little bit before merging. In the current PR state I've added FSharp.Control.AsyncSeq as a kernel dependency. I think it might be not needed.

I can prepare separate AsyncDisplay.fsx script that install FSharp.Async.Seq with Paket and regesters async printers after.

So to enable support of Async<_> and AsyncSeq<_> display the user will need to do #load AsyncDisplay.fsx

dgrechka commented 6 years ago

@cgravill I've separated out AsyncSeq dependency into separate .fsx file.

Checked that it works both ob Windows and on Ubuntu. Feel free to review and merge.

cgravill commented 6 years ago

It looks like you've got a single fsx. I think that's problematic between these two lines:

Paket.Package ["FSharp.Control.AsyncSeq"]

#load "Paket.Generated.Refs.fsx"

The failure is unfortunately only apparent when you delete the packages directory as would exist on a user machine. Could you please check and confirm if this is an issue? Or let me know if I've misunderstood!

I've tried a few ways to avoid the need for a double XYZ.paket.fsx and XYZ.fsx but haven't found a way yet. There's some longer term changes happening for #r that might help with this.

dgrechka commented 6 years ago

@cgravill You are right! There are problems on clean installation. Thank you! I've split the script into two. PR is updated.

Moreover. I found that after executing Paket.Package ["FSharp.Control.AsyncSeq"] on Ubuntu (Mono) the corresponding #r is not added into Paket.Generated.Refs.fsx main group script, So I can't use #load "Paket.Generated.Refs.fsx". As a workaround I've replaced it with direct #r "packages/FSharp.Control.AsyncSeq/lib/net45/FSharp.Control.AsyncSeq.dll"

Did you face it? Is it a Paket bug? Should we report these two bugs? 1) Inability to use single file 2) Absence of #r ... record in main group

cgravill commented 6 years ago

@dgrechka I think 1. is simply the way fsx files are designed to work (can you confirm @dsyme?), we're using .fsx in a way that's a bit unusual. I've tried a few approaches but haven't found a way to get it down to a single file - I thought I have previously but it didn't quite work consistently. It's possible there's a solution, and worth keep looking. I suspect that longer term this RFC https://github.com/fsharp/fslang-design/blob/master/tooling/FST-1027-fsi-references.md will be the full clear resolution.

The inconsistency in 2. might be down to Mono vs .NET with a framework target of NET471 as we have. Perhaps try validating with a separate project? Then you try reporting it as an issue.

Last small one, I suggest we use Paket.Version rather than Paket.Package to ensure updates don't break later.

cgravill commented 6 years ago

OK, looks good to me. Ready to merge from your side @dgrechka ?

dgrechka commented 6 years ago

@cgravill Yes. Conflicts are resolved. Proceed with merge.

cgravill commented 6 years ago

Excellent, this is a great foundation and already useful. I'll merge now.