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

Modal dialog for ipykernel installs & better messages #5798

Closed
DonJayamanne opened this issue May 6, 2021 · 24 comments
Closed

Modal dialog for ipykernel installs & better messages #5798

DonJayamanne opened this issue May 6, 2021 · 24 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug notebook-getting-started

Comments

@DonJayamanne
Copy link
Contributor

When running a notebook without ipykernel the user will be presented with the same message today, but modal diaglog as follows:
Screen Shot 2021-05-06 at 16 12 24

Should probably change the message, doesn't seem to be descriptive enought. Feels way too technical.
Someone starting jupyter notebooks or not aware of ipykernel will just ignore this.

It should ideally be along the lines:
In order to run notebooks IPykernel needs to be installed, do you want to install ipykernel?

@DonJayamanne DonJayamanne added the bug Issue identified by VS Code Team member as probable bug label May 6, 2021
@DonJayamanne DonJayamanne self-assigned this May 6, 2021
@rchiodo
Copy link
Contributor

rchiodo commented May 6, 2021

I still think it should reference the kernel though. And if it's too long, nobody will read it.

@claudiaregio
Copy link
Contributor

Going to keep it very simple

"IPykernel REQUIRED for this action"
Options: Cancel, Install

@rchiodo
Copy link
Contributor

rchiodo commented May 6, 2021

Yeah but that doesn't tell me where it's going, so I don't know if I want to answer that or not.

If it's too simple then I'm more afraid.

@DonJayamanne
Copy link
Contributor Author

"IPykernel REQUIRED for this action"

Isn't the assumption that users know what IPYKernel is, or does it not matter.

@DonJayamanne
Copy link
Contributor Author

t should reference the kernel though. And if it

Does this matter, i would have thought they'd need to know that its tied to their current operation (running cell in notebook/interaciev window).

I doubt if anyone looks at the kernel information (though thats very useful for diagnostic purposes) - because we fail to install in the right one.

@rchiodo
Copy link
Contributor

rchiodo commented May 6, 2021

At least the kernel name is something they can see elsewhere. And ipykernel can be looked up on google to find out what it is.

If we just say

'We need to install something - Yes or No'

Who is going to say Yes? My first question is what are you installing and second, where?

@DonJayamanne
Copy link
Contributor Author

Agreed, tha'ts why my suggestion
order to run notebooks IPykernel needs to be installed, do you want to install ipykernel?
But could be long

@claudiaregio
Copy link
Contributor

claudiaregio commented May 6, 2021

I do not think most users will know or really care too much what it is. The important message that needs to be given to users is "you are blocked if you do not install this".

We can keep the name so if users are curious they can do research to see what it is. I do not think this is the location where we need to explain to users in depth what is happening and why, it will be too long and they will not read it.

To @rchiodo 's point what we could do is have a learn more link lead to a doc in our github if people are so inclined to get details of what is being installed where. I believe most users would just click install when understanding this is required to continue.

@DonJayamanne
Copy link
Contributor Author

DonJayamanne commented May 6, 2021

@claudiaregio what's the message then, is it what you mentioned earlier with REQUIRED in caps?
That doesn't look right.

@claudiaregio
Copy link
Contributor

claudiaregio commented May 6, 2021

@DonJayamanne do you have a screenshot of what it would look like with all caps (basically how easy is it for me to see a mockup real quick :D)

@DonJayamanne
Copy link
Contributor Author

Screen Shot 2021-05-06 at 16 31 01

@rchiodo
Copy link
Contributor

rchiodo commented May 6, 2021

That seems awfully loud to me. I was just clicking a button and you're yelling at me?

@claudiaregio
Copy link
Contributor

how fancy can we get with these messages, can we underline?
I want to avoid that users just hit cancel but I guess if the message is short they are more likely to read it...

@rchiodo
Copy link
Contributor

rchiodo commented May 6, 2021

Personally I think this needs to be behind an experiment. You're making an assumption that people don't click the current install button because they miss it or they don't understand it.

To truly suss that out we should probably have 4 different ways:

  • Original
  • Original with modal
  • New text with error message in bottom
  • New text with modal

@claudiaregio
Copy link
Contributor

ideally we run one version against the one we have today which most people appear to "ignore", but if we have to a/b/n it we can do that too. I think I can narrow this down to one version to test

@DonJayamanne
Copy link
Contributor Author

DonJayamanne commented May 7, 2021

let's run this behind an experiment, ill write something up

Not sure we need an experiment for the modal dialog?
We can alraedy tell that users are ignoring this message. Not sure what we hope to get out of an experiment

I feel modal is mandatory, the question is whether the mesasge is meaningful enough and having an experiment seems wasteful to validate the text in a message.

@claudiaregio
Copy link
Contributor

Yeah running against one may not be worth it... ok. Let's just use a message of:

"IPykernel required for Python execution" @rchiodo no screaming ;)

Options:
Cancel, Install

@rchiodo
Copy link
Contributor

rchiodo commented May 7, 2021

We can alraedy tell that users are ignoring this message.

You're assuming they're ignoring it because it's not visible. That's why we're switching to Modal.

You're also assuming they're ignoring it because they don't understand it. And that a simpler message wouldn't be ignored.

I'm countering saying both are unknown.

My hypothesis is that people are ignoring the error because they don't want to actually run anything or they didn't realize that kernel was selected and they don't want to actually run with that kernel. This new message would make that worse as they still wouldn't know what kernel is selected but the message wouldn't tell them either.

The experiment is to ensure we aren't making things worse.

@greazer
Copy link
Member

greazer commented May 15, 2021

I don't think we need an experiment, yet. We are trying to find out whether users are simply not seeing the notification or whether they are just ignoring (by accident or on purpose). Putting the same message in a modal dialog box can certainly help answer the question as to whether they ignoring the message. To do so, we would just leave the message and operations the same, just make it modal.

However, if you try and "get into the heads" of some of our pure Jupyter customers it seems highly likely that many, if not most, of them will have zero familiarity with ipykernel itself. If so, that definitely would affect how they respond to any such notification regardless of whether it's modal or not. "I'm trying to use a Jupyter notebook. What does this ipykernel thing have to do with that?" I don't think we need an experiment to check this hypothesis either.

I don't think it hurts to make this message modal since the user is clearly trying to run a cell and simply will not be able to. Putting up a message to clearly indicate that they are blocked from what they are attempting to do seems completely reasonable.

With all that said, I would propose that we go ahead with a modal solution but change the message to say:

 "Running cells requires either Jupyter or the ipykernel package to be installed. __Learn More...__"

           "Install Jupyter" / "Install ipykernel" / "Choose different kernel" / "Cancel" 

"Learn More..." is a link or some "additional info" affordance to a page describing this problem.

I assume we can have 4 buttons. "Choose different kernel" isn't technically necessary and I'm not even sure if it's appropriate based on our current kernel finding algorithm. But I think it could help new users find an appropriate kernel without relying on them to find how to change it themselves, thus improving first-time success. It can also provide telemetry to help us understand whether users HAVE appropriate kernels available, but aren't getting selected correctly.

Note that if the user presses "Choose different kernel" then the kernel should be chosen and the run operation should be attempted automatically. If the new kernel still isn't good, then the same modal dialog would appear.

@DonJayamanne
Copy link
Contributor Author

I like this message, to me this is absolutely required "Running cells requires either Jupyter or the ipykernel package to be installed. __Learn More...__"
In my opinion majority of the users won't know what ipkernel is, hence will ignore them.

@claudiaregio
Copy link
Contributor

claudiaregio commented May 17, 2021

For !M1 + other users:
"Running cells with (name of interpreter) requires dependencies"
Options: "Install" , "Change Kernel"
Selecting Change Kernel should bring up the kernel quick pick options. After user selects a kernel

For M1 + others users:
"Running cells with (name of interpreter) requires Jupyter"
Options: "Install" , "Change Interpreter"

@rchiodo
Copy link
Contributor

rchiodo commented May 18, 2021

No learn more button? I'm not just going to install 'stuff' on my machine.

@claudiaregio
Copy link
Contributor

For !M1 + other users:
If pip installed: "Running cells with (name of interpreter) requires ipykernel"
If pip not installed: "Running cells with (name of interpreter) requires ipykernel and pip"
Options: "Install" , "Change Kernel"
Selecting Change Kernel should bring up the kernel quick pick options.

For M1 + others users:
If pip installed: "Running cells with (name of interpreter) requires Jupyter"
If pip not installed: "Running cells with (name of interpreter) requires Jupyter and pip"
Options: "Install" , "Change Interpreter"

Adding details on what is being installed so users can look up what they are about to add to their machine without having to add a learn more button

@rchiodo
Copy link
Contributor

rchiodo commented Jun 2, 2021

Validated

@rchiodo rchiodo closed this as completed Jun 2, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 10, 2021
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-getting-started
Projects
None yet
Development

No branches or pull requests

5 participants