Skip to content

Excel integration#255

Merged
zyzhu merged 5 commits intofslaborg:masterfrom
dcharbon:ExcelExperiment
Aug 4, 2018
Merged

Excel integration#255
zyzhu merged 5 commits intofslaborg:masterfrom
dcharbon:ExcelExperiment

Conversation

@dcharbon
Copy link
Copy Markdown
Contributor

No description provided.

dcharbon added 5 commits July 26, 2014 11:24
A Deedle Frame can be sent to a range in an Excel spreadsheet. Further,
the Frame may optionally be automatically synced from FSharp to Excel
(one-way by update in FSharp) by wrapping the Frame in a KeepInSync
object.

Additional changes were made to get the tests passing, along with the
build, by adopting the later revisions of dependencies.
@tpetricek
Copy link
Copy Markdown
Member

Sorry for not getting to this earlier. This is a feature that we definitely should merge, but I think the current pull request could use some cleanup. Which of the following sounds better:

  1. Merge the PR as is and treat Excel integration as experimental (just like Big Deedle). We can tweak the API and add documentation later.
  2. Do more development in a separate branch and then merge into the main Deedle branch once the feature has all it needs.

What I think is missing from the current PR:

  • I find the API somewhat confusing and there are too many ways for doing things (e.g. you can say xl?A1 <- frame, you can use frame |> Xl.AsTable(), but there are also some other globals like saveToImage, the *= operator (??) and DeedleToExcel function). I think we need to think a bit about the different use cases and clean up the API a little.
  • It need some documentation - currently, the sample script demonstrates only xl?A1 <- frame and it would be good to turn this into a proper part of the documentation
  • In principle, it would be nice to have some tests, but this is probably not going to be easy (Excel...) so we can live without that

.Should we go with (1) or (2) ?

@tpetricek
Copy link
Copy Markdown
Member

Two more thoughts:

  • It would be nice if the Excel integration could be loaded on Mono - it does not have to work, but #load on the file should not cause errors (I have not tried this - so this might be fine)
  • We have Deedle.RPlugin and Deedle.Excel packages, but does it work if people want both? (Have not tried it and I think it does, but it is good to check...)

@dcharbon
Copy link
Copy Markdown
Contributor Author

I prefer option (1) and agree with your points. I can give both loading on mono and loading both Deedle.RPlugin and Deedle.Excel a try this week, too.

@dsyme
Copy link
Copy Markdown
Member

dsyme commented Jul 27, 2018

@dcharbon Could you assign myself and @zyzhu as owners of the Deedle.Excel package please? https://www.nuget.org/packages/Deedle.Excel/1.0.4-rc1

@dcharbon
Copy link
Copy Markdown
Contributor Author

@zyzhu what is your nuget username? Is it the same as your github username?

@zyzhu
Copy link
Copy Markdown
Contributor

zyzhu commented Jul 27, 2018

@dcharbon I just registered. It's the same as GitHub. Thanks!

@zyzhu zyzhu merged commit 7386010 into fslaborg:master Aug 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants