tmastny / sagemaker

AWS Sagemaker R Interface
https://tmastny.github.io/sagemaker/
Other
7 stars 0 forks source link

sagemaker_install() fails, can't find sagemaker package #3

Closed jcpsantiago closed 4 years ago

jcpsantiago commented 4 years ago

Great initiative!

I tried sagemaker_install() and it fails because sagemaker is not available in Conda. Instead I had to install sagemaker-python-sdk. Do you have configuration that points sagemaker_install() to another source (e.g. PyPi)?

tmastny commented 4 years ago

Using ... in sagemaker_install, you can send any additional configuration options to reticulate::py_install.

You can use the pip argument of py_install to change the installation method. By default it is false, and since method = "auto" by default as well, it tries to install via Conda for you.

However, I think what you bring up is a real bug. I didn't realize that the Sagemaker package was called sagemaker-python-sdk for Conda and just sagemaker for PyPi.

My current implementation naively hardcodes in sagemaker for either installation method: https://github.com/tmastny/sagemaker/blob/4d835ef3f3fbfa64a2ac5dd616eeb491ed3a3f39/R/install.R#L15-L17

The easiest fix will be to have two separate functions, sagemaker_install_pypi and sagemaker_install_conda. Otherwise I'd have to figure out a way to determine if the user wants Conda or PyPi, so I can use the right package name to install.

tmastny commented 4 years ago

Or maybe the event simpler way is just to force users to install the Conda version 🤔.

jcpsantiago commented 4 years ago

at least for me reticulate offered to install miniconda, that's why I have it. Maybe it's its default?

If yes, then assuming Conda is installed would be safer IMO. There is no s3 package in Conda either btw :D