Remove requirement of SparkR

jupyter / declarativewidgets

[RETIRED] Jupyter Declarative Widget Extension

http://jupyter.org/declarativewidgets/docs.html

Other

120 stars 38 forks source link

Remove requirement of SparkR #533

Closed wjsi closed 7 years ago

wjsi commented 7 years ago

As R users are not always using SparkR, it is better to activate the Spark DataFrame serializer only when SparkR is installed.

BTW, check_packages() in declarativewidgets/kernel-r/declarativewidgets/R/serializers.r always returns TRUE. It is also fixed in this PR.

poplav commented 7 years ago

@siyuwj

That is a good point. I see a few things that can be done:

In load_serializers we should first check the serializer has it's dependencies via check_packages. This is a bit cleaner than the initial commit for this PR and is what the python side does here. We should also add logging that the package failed to load and the serializer was not inited correctly.
The tests directly use sparkR also, so those should be separated out and logged that sparkR type tests were skipped due to unmet dependencies.

wjsi commented 7 years ago

@poplav I added check_packages in register_serializer. Spark tests are skipped when SparkR is not installed.

poplav commented 7 years ago

@siyuwj I think your changes look good, we are having issues with tests at the moment so bear with us regarding timing of merging.

poplav commented 7 years ago

@siyuwj Thanks again for your contribution. We recently finished a revamp of our testing/build process at https://github.com/jupyter/declarativewidgets/pull/532. Can you rebase this with master and then we'll merge it!