search

fslaborg / Deedle

Easy to use .NET library for data and time series manipulation and for scientific programming
http://fslab.org/Deedle/
BSD 2-Clause "Simplified" License
933 stars 195 forks source link

Handle F# Data dependency (for CSV parsing) via Github #289

Closed tpetricek closed 9 years ago

tpetricek commented 9 years ago

This removes the dependency on FSharp.Data package. We can now reference the files we need directly from F# Data Github using a new nifty Paket feature.

The benefit is that we have a smaller stand-alone package that can be combined with any version of F# Data and also is less prone to binding redirect errors (no MethodMissingException when using wrong FSharp.Core.dll).

The only slightly tricky thing is that we need to find a closed set of files from F# Data which covers all the functionality we need. This includes the type inference & CSV parsing, but there are other helpers (e.g. for IO). Since I did not want to include all files, just because of a few unused helpers, I included Stubs.fs with three functions that I did not want to copy from F# Data.

[This adds more things on top of #288, so the diff will be nicer after that one is merged.]

tpetricek commented 9 years ago

The most recent commit addresses the comment by @dsyme. No internals of F# Data will now be exposed by Deedle.

Aside from the changes in how we reference things in Paket, the only other change in this PR is that I simplified one unit test so that it does not use F# Data (as we don't really need that).

@adamklein @hmansell I think this is now ready for your review! (Also cc @dsyme @ovatsus @forki in case they have some thoughts on this...)

adamklein commented 9 years ago

Code looks good. Was trying to compile on VS2013 (got a new box, lost my VS2012, might not be set up correctly, first time trying to compile Deedle on this new machine) and get an 

1>C:\code\Deedle\src\Deedle\Deedle.fsproj(135,5): error MSB3030: Could not copy the file "..\..\packages\FSharp.Data\lib\net40\FSharp.Data.DesignTime.dll" because it was not found.

Am I missing something silly?

forki commented 9 years ago

I have to investigate if @tpetricek added VS package restore. If not then this is the issue.

Did you run build.cmd? It will download all the packages. (this should be a workaround to get you working again)

adamklein commented 9 years ago

@forki @tpetricek Confirmed everything works through build.cmd, and that following executing this script, things compile fine in VS2013.

forki commented 9 years ago

Ok I will try to enable VS package restore in separate PR

forki commented 9 years ago

Seems it's not the VS package restore process. That thing works.

Strange thing is: why does it look for "packages\FSharp.Data\lib\net40\FSharp.Data.DesignTime.dll"? I thoght we removed that here.

I have an idea but still need some investigation

forki commented 9 years ago

see https://github.com/BlueMountainCapital/Deedle/pull/293

tpetricek commented 9 years ago

I think the project for evaluating performance of Deedle still uses F# Data, so the package is still referenced via Paket, but it should not be needed (or accessed) when just building Deedle. I'll try from a clean clone....