Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow users to create a requirements.txt to install missing packages #68

Merged
merged 5 commits into from
Oct 31, 2020

Conversation

raman325
Copy link
Contributor

@raman325 raman325 commented Oct 31, 2020

As I mentioned in #66, it would be nice to be able to have pyscript install packages to be used in imports in cases where the user doesn't have the ability to install the packages themselves (in my case, I am using a Docker container for HomeAssistant so it would be cumbersome to install these requirements on every container rebuild).

I went ahead and implemented the functionality in this PR for your consideration. The way it works is that users can add a requirements.txt file to pyscript/. For any packages that are already installed, the version will be ignored so as not to mess with HomeAssistant functionality, and for any packages that aren't already installed, the package will be installed via a Home Assistant function. Since I am leveraging the Home Assistant function, the actual work being done within pyscript is fairly minimal.

This activity to install requirements will be done on initialization and on reload.

Resolves #66

@raman325 raman325 force-pushed the install_packages branch 2 times, most recently from 5ddb7b6 to 013be73 Compare October 31, 2020 03:18
@raman325
Copy link
Contributor Author

As I was thinking through this, there is a "slicker" way to do this, which would be to add the logic to install the module during import time (https://github.com/custom-components/pyscript/blob/master/custom_components/pyscript/eval.py#L843 and https://github.com/custom-components/pyscript/blob/master/custom_components/pyscript/eval.py#L876). The advantage of this approach is that the user doesn't have to add a requirements.txt or do anything really to modify their code, we could just check that the module is available and install it if not. The disadvantage is that there would be no way to pin to a version. I personally prioritize stability over convenience, which suggests that my original approach is better, but I am presenting that option in case you are against the idea of adding another file to manage and value convenience over stability (stability coming from the ability to pin a version).

@dlashua
Copy link
Contributor

dlashua commented Oct 31, 2020

I think I prefer the requirements.txt approach. Not only does it allow pinning a version, but, if we're installing a module, it's best to be explicit about that. This approach will provide the least amount of surprise. Especially if someone requests a module that takes forever to build (like numpy) and doesn't have a prebuilt binary available.

custom_components/pyscript/__init__.py Outdated Show resolved Hide resolved
custom_components/pyscript/__init__.py Outdated Show resolved Hide resolved
custom_components/pyscript/__init__.py Show resolved Hide resolved
custom_components/pyscript/__init__.py Outdated Show resolved Hide resolved
custom_components/pyscript/__init__.py Outdated Show resolved Hide resolved
@craigbarratt
Copy link
Member

craigbarratt commented Oct 31, 2020

I definitely like this approach using requirements.txt. Implicitly installing packages at import time is undesirable for several reasons - Python doesn't do it, it could be confusing, and it doesn't allow version pinning as @dlashua noted.

I added a few comments to the PR, which is looking good.

One request is to support multiple requirements.txt files in the following directories:

  • pyscript/requirements.txt: top-level already supported
  • pyscript/apps/APP/requirements.txt: if an app uses the package (directory) form, it can optionally specify its own requirements. If you use the simple module form pyscript/apps/APP.py then you can't specify requirements.
  • pyscript/modules/MODULE/requirements.txt ditto for modules.

The motivation for the last two is it allows apps or modules to be self-contained and they will just do the right thing when installed.

As an example, you can see how load_scripts() uses glob.glob() to find files to load

@craigbarratt
Copy link
Member

This all looks great. Merging. Thanks!

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.

Feature Request: Support python package installation somehow
3 participants