-
-
Notifications
You must be signed in to change notification settings - Fork 393
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
show python requirements not found message and option to install #712
Conversation
I started testing it, but did not get to it - I will report back tomorrow. |
So when I tried to open an environment with an older version of JupyterLab:
I got this distressing screen: which has no environment picker so I cannot change the environment (without restarting app). I was able to use the hamburger menu to go to settings and grab the logs using the new "Show logs" button.
Note: running the command manually in the environment works fine, so I am not sure what is the error. In any way, the point is that there might be something wrong with version checks for Jupyter Server if it does not work? |
@krassowski was this environment shown in your environment selection list on top right or did you browse the python path to select it? This failure is not due to version incompatibility, running the |
It's both. I do not have any suggestions in the environment list auto-populated because I use pyenv so I always have to browser for path to select it (hence I wish there was auto-detection for non-conda environments). But once I select it it stays in the environment selection list forever. It used to work previously, so I guess there may be something broken. |
I tried another environment which starts up fine with jupyterlab 3.6.3. My hypothesis was that it was the older JupyterLab version, so I downgraded. After downgrading the jupyterlab in the environment the picker tooltip still says that it sees 3.6.3. This is odd. How does it get jupyterlab version? |
Environment metadata is updated after a rediscovery, it should update the version shown in the list after few seconds from launch. |
I tried in a conda environment with the same versions you posted above, and it worked just fine. jupyterlab >= 3.0.0 is supported |
Ok, I know how to repro it reliably. It is a conflict of versions problem - it was happening when I stated So this is not a typical failure scenario, but it does show that this error screen needs work (in particular it would be lovely if it had the environment selector). |
const requirementInstallCmd = encodeURIComponent( | ||
this._registry.getRequirementsPipInstallCommand() | ||
); | ||
message = `Error! Required Python packages not found in the selected environment. You can install using <copyable-span label="${requirementInstallCmd}" copied="${requirementInstallCmd}"></copyable-span> command in the selected environment.`; |
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.
Is it possible to show the detected version here>
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.
it is possible. it is easier to just log them though.
src/main/registry.ts
Outdated
this._requirements = [ | ||
{ | ||
name: 'jupyterlab', | ||
moduleName: 'jupyterlab', | ||
commands: ['--version'], | ||
versionRange: new semver.Range(`>=${minJLabVersionRequired}`) | ||
versionRange: new semver.Range(`>=${MIN_JLAB_VERSION_REQUIRED}`), | ||
pipCommand: 'jupyterlab' |
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.
should it include >=${MIN_JLAB_VERSION_REQUIRED}
as well?
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.
it was causing installation to fail on Windows. I will take another look.
src/main/registry.ts
Outdated
@@ -446,6 +468,16 @@ export class Registry implements IRegistry, IDisposable { | |||
return this._requirements; | |||
} | |||
|
|||
getRequirementsPipInstallCommand(): string { |
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.
I am afraid that suggesting pip
install will lead to many problems if novice conda users end up executing this in their env. Worst case scenario is having two conflicting versions with assets from one and scripts form another which is quite common problem but hard to debug.
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.
is this an issue when an old jupyterlab is installed with conda install
, then updated with pip install
. I thought pip install was fine in a conda environment.
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.
It will only work when NOT in the base environment (ideally in isolated environment) and if pip
was explicitly installed. Many users fall into this trap.
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.
Of course if you install the same version in base and leaf env, one using pip and the other using conda, it does not matter. Up until the very moment one gets updated and then it breaks.
@krassowski could you take another look as I pushed some updates addressing your comments. |
I think I am seeing an issue with
but this does not seem to work now? I am not 100% positive if this is due to this PR or because some setting got lost during uninstall/install sequence, but given that this PR touches python path logic it could be related. This is kind of a deal breaker for me, but possibly not a common usage pattern. Note that configuring this in settings does not seem to work either. Is Python path overwritten at some point, or am I fooling myself? I am going to try an older version now. |
I tested both from CLI like your example and also from the settings dialog and both worked fine. This PR doesn't make any changes to environment variable setting. |
Thank you for looking into this! Apologies for the delay with the actual review, will take another look soon. |
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.
there is actually a subtle animation of the orange part of the logo, like heart beat. but probably too subtle to notice. |
jupyterlab
are not found.copyable-span
web component to support copying to clipboard from web views