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

Move installation of all DS packages into Jupyter Extension #8457

Closed
DonJayamanne opened this issue Dec 4, 2021 · 5 comments · Fixed by #9228
Closed

Move installation of all DS packages into Jupyter Extension #8457

DonJayamanne opened this issue Dec 4, 2021 · 5 comments · Fixed by #9228
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug notebook-kernel Kernels issues (start/restart/switch, install ipykernel)
Milestone

Comments

@DonJayamanne
Copy link
Contributor

DonJayamanne commented Dec 4, 2021

  • We're already install ipykernel for windows PCs when users have shells other than Powershell/Command Prompt
  • We've extended this to now install ipykernel ourselves always when on Windows PC (global & virtual environments)

Suggestion

  • Install ipykernel for conda enviroments ourselves
    • We can actiavte the env and run conda install ipykernel ourselves (we have the location for conda, etc)
  • Instlal ipyernel for other environmetns (pipenv, poetry, etc) ourselves
    • We can activate the env using Python extension, and run the pip installer

This should fix a number of ipykernel installation issues as well.

Benefits

  • More users will be unblocked
  • We have a better idea/understanding of when things go wrong
    • Today when things go wrong, we have absolutely no idea in Python ext, because we run the command in the terminal and we don't know what goes on in the terminal
    • (also there's no need to run these in a terminal)
  • Simpler
  • Lower reliance on Python ext (we can just rely on getting the type of env)
@DonJayamanne DonJayamanne added bug Issue identified by VS Code Team member as probable bug notebook-kernel Kernels issues (start/restart/switch, install ipykernel) labels Dec 4, 2021
@rchiodo
Copy link
Contributor

rchiodo commented Dec 4, 2021

The downside I see is any changes we need because conda changes, we'll be duplicating what Python is doing. Unless they stop doing this altogether, seems like we should make an npm module for this?

@IanMatthewHuff
Copy link
Member

Not that this should influence our choice totally. But does the python extension want to own this type of thing (package installing) for the VS Code python extension ecosystem? If they do then it might be worth trying to solve problems there in the python extension. If they don't (and this was more exposed just for our team) then sure why not do this ourselves?

@DonJayamanne
Copy link
Contributor Author

doing this altogether, seems like we should make an npm module for this?

Python will be removing all code related to installation, hence there won't be much duplication.
Also the only code that we duplicate is conda install (that's the command that is duplicated).
E.g. with teh current code we're running python -m pip install, thats already technically a duplication (but that's a single line of code).
Hence I'm not sure this is a duplicatoin.

I agree, we can create an npm module, as part of this. shoudl be super easy & i propose we create that npm in this same repo.

@DonJayamanne
Copy link
Contributor Author

But does the python extension want to own this type of thing (package installing)

I thought they don't want to own this.
& us waiting will not get us anywhere, we've been waiting for this to happey while users break and stop using.
I think we can go ahead with our npm, and when things get traction in python extnsion, we can consider merging.

Again, the only duplication is literally a few lines of code.
& the Python extnesion doesn't really have the smarts today, it just dumpts the commands into a terminal (& that will go away as they don't want to do any of this in the future), our code will be slighly different.

@DonJayamanne
Copy link
Contributor Author

Again, I don't think there's any code duplication, but totally fine with the npm module.
Should be super easy.

@rchiodo rchiodo added this to the January 2022 milestone Dec 6, 2021
@rchiodo rchiodo self-assigned this Feb 11, 2022
@greazer greazer modified the milestones: February 2022, March 2022 Feb 24, 2022
@rchiodo rchiodo mentioned this issue Mar 21, 2022
2 tasks
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug notebook-kernel Kernels issues (start/restart/switch, install ipykernel)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants