feat: implement importing excel (.xls .xlsx) files

[shun2wang]

Close: jasp-stats/jasp-issues#2134
Close: https://github.com/jasp-stats/INTERNAL-jasp/issues/1390
Close: https://github.com/jasp-stats/INTERNAL-jasp/issues/412

JASP can now import excel files from first workbook and the first sheet by default.

I introduced a third-party library freexl to parse the excel files. but a known limitation is that you cannot import files with more than 705 columns (this is a bug of freexl), I made a patch to lifte this limitation temporarily.

• init implement import excel files (on Linux)
• work for Windows
• work for macOS (someone could help?)
• update development document

My C++ skills are not yet proficient. I would be grateful for your guidance and review.

[boutinb]

What nice!!! I'll try it tomorrow on Mac.

[boutinb]

I wonder why you did not use conan to load the freexl library.
I have done this for Mac (see my last commit), and it works perfect. we don't need to copy the freexl.h file in this way.
Can you try this on Windows and Linux?

[shun2wang]

official FreeXL has a bug, so we using a patch version from my fork.

[shun2wang]

I wonder why you did not use conan to load the freexl library. I have done this for Mac (see my last commit), and it works perfect. we don't need to copy the freexl.h file in this way. Can you try this on Windows and Linux?

because it cannot read more than 700 cols Excel files so I made a patch and build it manually. see my first comment.

see: https://github.com/shun2wang/freexl

[shun2wang]

see: https://github.com/shun2wang/freexl

Do we maintain a repo of JASP, I am just using my fork for now. @boutinb

[boutinb]

But if I see your PR, you are using a freeXL version set in rtools.
We could use your freeXL version on GitHub and compile it, but for maintenance reason (JASP depends on many libraries, and its build is becoming quite complex) I would prefer to lie on conan which takes care on portability and compiling issues.
That the current freeXL cannot load more than 700 columns is not a big issue I find. I would ask freeXL to merge your fix (maybe you did it already), so that the next version will include your fix.

[shun2wang]

But if I see your PR, you are using a freeXL version set in rtools.

Yes, we using rtools msys2 build toolchain to build it on Windows and the ReadStat also build manually by this way. maybe @JorisGoosen know more details here.

That the current freeXL cannot load more than 700 columns is not a big issue I find. I would ask freeXL to merge your fix (maybe you did it already), so that the next version will include your fix.

We have received complaints from users about multi-column importing, and Rens has applied a fix temporarily for that. Yes I had filed bugs to that project but I dont think it will be merged for quite some time, because freeXL is updated very infrequently to ensure stability, and the last release was a year ago.

Your concerns are valid, I would try to find a way to apply patches manually from conan instead of waiting for the official master branch release.

EDIT: no, I used a dev branch of FreeXL which including some bugfix important. I think maybe build it manually is best choice for now.

[JorisGoosen]

I agree with Bruno that adding more manual stuff to the build is not very maintainable...
And Im already unhappy with the fact that for some systems we need to manually build readstat on some systems.

But if I see your PR, you are using a freeXL version set in rtools.

Yes, we using rtools msys2 build toolchain to build it on Windows and the ReadStat also build manually by this way. maybe @JorisGoosen know more details here.

That the current freeXL cannot load more than 700 columns is not a big issue I find. I would ask freeXL to merge your fix (maybe you did it already), so that the next version will include your fix.

I agree that staying under 700 columns until they merge your fix is not at all a problem.
Just making it possible to import from xlsx files etc is going to be a highly appluaded feature in any case.
So very nice you made it.

We have received complaints from users about multi-column importing, and Rens has applied a fix temporarily for that. Yes I had filed bugs to that project but I dont think it will be merged for quite some time, because freeXL is updated very infrequently to ensure stability, and the last release was a year ago.

Your concerns are valid, I would try to find a way to apply patches manually from conan instead of waiting for the official master branch release.

EDIT: no, I used a dev branch of FreeXL which including some bugfix important. I think maybe build it manually is best choice for now.

What bug was fixed?
Maybe it is ok to just use the version with bug that is in conan?

I will now do an indepth review of your code ^^

[JorisGoosen]

Also, if we are going to use our modified version I would like to move that code to jasp-stats and use that one.

[JorisGoosen]

Already a thing: in qml and c++ files we use tabs not spaces. So perhaps you could clean up your indentation to be tabs of 4 instead of the spaces?

Edit: I see that you do use tabs in a lot of places, just not everywhere?

[shun2wang]

Thanks for your reviews! OK let's use conan version to make build easier. and ignore 700 cols limitation. then I will following the official release. 🍻

[boutinb]

It works really well on Mac.
I think we can merge this for 0.19.2

[JorisGoosen]

I agree!

Very nice @shun2wang 😃