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

Remove "New Python Installation" option in Configure Python dialog #25569

Merged
merged 11 commits into from
Apr 10, 2024

Conversation

corivera
Copy link
Member

@corivera corivera commented Apr 10, 2024

The current provided python package has gotten pretty old and is nontrivial to update, so we decided to deprecate the New Python Install option. We already automatically detect installed versions of python and also provide a guided experience to install required pip packages, so the only work a user would need to do is install python on their machine before using jupyter notebooks. I improved the automatic version detection as well, so it picks up more recent versions of python.

I also cleaned up some unused jupyter code in the machine-learning extension at the same time, since I already had to make some changes in that area as part of this larger work.

Screenshots of new dialog UX:

  1. With python already setup
    EditPath

  2. When specifying different versions
    PathDropdown

@coveralls
Copy link

coveralls commented Apr 10, 2024

Pull Request Test Coverage Report for Build 8637291071

Details

  • 7 of 12 (58.33%) changed or added relevant lines in 4 files are covered.
  • 60 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.02%) to 41.739%

Changes Missing Coverage Covered Lines Changed/Added Lines %
extensions/notebook/src/dialog/configurePython/configurePythonWizard.ts 0 5 0.0%
Files with Coverage Reduction New Missed Lines %
extensions/notebook/src/dialog/configurePython/configurePathPage.ts 1 58.06%
extensions/notebook/src/jupyter/jupyterServerInstallation.ts 59 48.9%
Totals Coverage Status
Change from base Build 8619232124: -0.02%
Covered Lines: 30721
Relevant Lines: 68888

💛 - Coveralls

@corivera
Copy link
Member Author

One question I had is whether it's worth it to still have that first "Edit" UX. Before it was used to cover up all the controls for New/Existing install to reduce clutter when python had already been configured, but now the page looks basically the same either way.

@kisantia Your thoughts?

@kisantia
Copy link
Contributor

One question I had is whether it's worth it to still have that first "Edit" UX. Before it was used to cover up all the controls for New/Existing install to reduce clutter when python had already been configured, but now the page looks basically the same either way.

@kisantia Your thoughts?

To make sure I'm understanding the question - is it if the first screenshot scenario should be removed and it just always be the second screenshot?

@corivera
Copy link
Member Author

One question I had is whether it's worth it to still have that first "Edit" UX. Before it was used to cover up all the controls for New/Existing install to reduce clutter when python had already been configured, but now the page looks basically the same either way.
@kisantia Your thoughts?

To make sure I'm understanding the question - is it if the first screenshot scenario should be removed and it just always be the second screenshot?

Correct

@kisantia
Copy link
Contributor

One question I had is whether it's worth it to still have that first "Edit" UX. Before it was used to cover up all the controls for New/Existing install to reduce clutter when python had already been configured, but now the page looks basically the same either way.
@kisantia Your thoughts?

To make sure I'm understanding the question - is it if the first screenshot scenario should be removed and it just always be the second screenshot?

Correct

That sounds good to me, especially if it helps simplify the code too. Any thoughts on this change @chlafreniere?

@corivera
Copy link
Member Author

I'll make that change in a subsequent PR to keep this one from getting more complicated.

@corivera corivera merged commit 04e1fb7 into main Apr 10, 2024
14 checks passed
@corivera corivera deleted the corivera/removePython branch April 10, 2024 22:08
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.

None yet

4 participants