-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Improve our error messages for non compatibility. #9900
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
Conversation
@willingc I would appreciate any of your insight on the error message. I'd like @fperez and @ellisonbg input on this process. I even think that a real multine error that also give some (basic) reasoning as why we stop support, and how to get the 5.x branch woudl be welcome. |
I think this PR is already a big improvement. I wonder if it would be useful to host a permanent page with a bit more detail, and include a bit.ly URL to it in the message. Something like But this is already a good start! |
I think we can do better, I don't know if we have a bit.ly account, and that would be fine with me to have an updatable page. .
|
Also once we agree on the message, we need to duplicate it in setup.py. |
I'm just not convinced that an exception traceback is the place for long-form instructions. That's why I think that it's better to point to the source of the problem and link to a persistent resource with more details. Putting tutorials into tracebacks is, I think, stretching too far the intent of an error message. |
if sys.version_info < (3,3): | ||
raise ImportError('IPython 6.0+ requires Python version 3.3 or above. ' | ||
'For a version compatible with Python 2.7, please install' | ||
' the 5.x Long Term Support version.') |
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.
raise ImportError('IPython 6.0+ does not support Python 2.7, 3.0, 3.1, or 3.2. '
'When using Python 2.7, please install IPython 5.x LTS Long Term Support version. '
'Beginning with IPython 6.0, Python 3.3. and above is required.')
Since the error message will be going to someone that does not have 3.3+ installed, the above error message using the following: 1. Tell them the error. 2. Tell them the resolution (use 5.x LTS). 3. Tell them why.
That's one of my other questions, should we only raise (just for imports), or should we |
Perhaps for simplicity, it would be good to have a brief error message as I suggest in the code review. One sentence that could be added to the error message that refers to a changelog. For example,
We could add a CHANGELOG.md file at the repo's root with @Carreau's longer error text re: IPython 6.x and link to the docs from there. The CHANGELOG.md would be easy to update as needed. |
A Changelog file is really tricky to maintain in a distributed VC environment as it easily leads to merge conflicts. And I'd rather not start a ChangeLog again (we used to have one), since now our release process instead generates its equivalent out of the github PR and issue list. But, iterating on this idea: how about instead we add a short section on this topic at the end of the main README.md? This is likely important enough to warrant mention there for a while to come, and that would mean it's available to the users always right next to them (if they are installing from source, by definition they have the README.md). So, we could keep the short & sweet error message as per @willingc's suggestion in the code, and a bit more info at the end of the README. That would, I think, satisfy the constraints, obviate the need for managing a separate URL, and pick up on the Changelog idea without the issues that specific kind of file brings up. Thoughts? |
Done. |
Iteration FTW. @fperez Great suggestion. |
Thanks ! |
Great, thx! |
I would like to have the transition (even on master) as smooth as possible, with good error message.
There is a large number of instruction that explain how to install ipython from source on the internet, and we can make an effort for all the python 2 users around that will encounter that to have a seamless transition or at least a good error message.
Otherwise we will get a lot of bug report from users.
One of the thing we can do, is to actually open an issue just for that, and point to this issue number in the error messag, plus update this issue with instructions.
The following is not super friendly: