-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Implement cases for pipenv and poetry module installers for the DataScienceInstaller #16616
Conversation
FWIW, I believe the data science team intentionally only implemented support for installing via |
Both the Pipenv and Poetry installers are run with the Also, running pip within a poetry environment will work, but it puts the lock file out of sync and causes other issues. |
Tagging the data science engineers as FYI since I believe our team owns the data science-specific parts of the Python extension codebase 😊 |
LGTM from data science side. Final signoff should be from the Python folks though. |
@kimadeline can you review for the Python extension team please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tonybaloney PR looks good, please fix the TypeScript tests and we can merge it afterwards. |
@kimadeline oops. fixed it. |
Fixes #16615
There are 4 module installers:
I noticed that the names of the Pipenv and poetry installers are constants, so I've imported those constant values instead of hardcoding the string (like it is with the
Pip
string)