Skip to content

Conversation

@dfalbel
Copy link
Member

@dfalbel dfalbel commented Apr 2, 2019

This will add flow_images_from_dataframe function. Fixing #658.
Needs more testing before merging.

@dfalbel dfalbel marked this pull request as ready for review April 3, 2019 00:18

test_succeeds("flow images from dataframe works", {

if (!reticulate::py_module_available("pandas"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the function itself requires pandas we should note this in the documentation (and also do a check during the call to print a clear error message to that effect). Note that pandas will generally not be installed in TF environments installed via install_tensorflow() without special extra effort.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


if (!reticulate::py_module_available("pandas"))
stop("Pandas (python module) must be installed in the same environment as Keras.",
'. Install it using reticulate::py_install("pandas", envname = "r-tensorflow").')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does leave open whether they used virtualenv or condaenv to install (the code above will default to virtualenv).

They could also do this: install_tensorflow(extra_packages = "pandas"). Not sure if that's a better thing to document or not though (will leave that to your discretion!)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jjallaire
Copy link
Member

jjallaire commented Apr 3, 2019 via email

@dfalbel dfalbel merged commit 2597582 into rstudio:master Apr 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants