-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Add additional diagnostics to our error messages #10421
Comments
+1 from me, I guess. Presumably the intent is to keep this as an umbrella issue, kept open indefinitely, with individual PRs linked to it as we change messages? Would a project be better for this? Or are you thinking of this as being about implementing a more structured "generate an error" API, a bit like the deprecation helper? I imagine there will be some low-hanging fruit here along with some much trickier ones (ahem resolver). |
Well, mostly to gain consensus before I try tackling this in a more structured manner; similar to the documentation rewrite. I'll likely end up doing the same thing -- keep the issue open and backlink to this; with a project for tracking progress. |
Cool, I see no problem with doing this, it seems like it can only improve the UX when things go wrong. |
I'm also +1 on this change. Doing this can also reduce the number of issues based on obscure error messages. WIth clear messages, the user may understand what to do, instead of opening a new GitHub issue :) |
If anyone has suggestions for places to improve or wants to cross-reference existing issues to this one, I’m all ears for that. The more potential enhancements we can find that fit this, the better I’ll be able to figure out what to spend time on! :) |
#8262 (comment) is an example. Explaining that the error is from the package rather than pip would help. I also wonder if tweaking pip-crashed tracebacks to be more distinct from regular Python tracebacks would help in these situations. |
#6584 as well! |
#8738 is another. |
Looking at #8618, it looks like the failure to build packages trips a lot of new users as well! |
Alrighty. So, here's the things that I've spent basically all day thinking through today:
I'll try to go through and summarise my thoughts on each of these one-by-one. This is definitely open to all kinds of feedback and inputs; and I'm likely gonna throw out a tweet or two to get the broader community to wieght in on this as well. Here's the presentation style I've settled on: (output streams that will be happy with unicode)
(all the other "lame" streams)
Note that the {reference} ids in this can be made into hyperlinks as well, when supported -- https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda -- and it would be pretty neat to link to our documentation from this. :) As an example, here's what we've go today:
Here's how that'll look with this model:
That hint at the end is triggered by "Is number of available distributions for this zero?", which... well... it's quirky to get that to be shown only in the right situations. :( Here's another example, demonstrating the issue with that:
With the diagnostics model I'm suggesting here, we'd present the same information in the following format instead:
If we can manage adding some complexity, for passing the information about the "get-page" failure, to the point in the code where we actually raise the "No matching distributions" error -- we can do something like:
And, perhaps most importantly, I still need to figure out how to get pretty colours working in this model. This might need something drastic like #10423 to be done though. 🤷🏽 This also makes progress on a few blanket issues/broader ideas we have:
This also interacts well with:
I'm still thinking about whether this should live as code within pip, or be a separate dependency that we vendor. I'm leaning toward the former, since I don't want to take on the maintainance overhead of providing such a package more broadly (OTOH, it'd be REALLY NICE!). Anyway, it is definitely possible to implement things in a way that keeps our options open, and I intend to do that. The idea is essentially to introduce a This roughly translates to looking with the following shape: class DiagnosticPipError(PipError):
reference: str
message: str
kind: Literal["error", "warning"]
reference: Optional[str]
attention: Optional[str]
hint: Optional[str]
class MetadataGenerationFailure(DiagnosticPipError):
reference = "metadata-generation-failed"
def __init__(self, failure: Any) -> None:
# Do checks on the failure to figure out the right values here.
super().__init__(
message=...,
context=...,
attention="This is likely an issue with how the package has to be built/installed.",
)
try:
...
except DiagnosticPipError as exc:
present_diagnostic(exc) The things that we'd need to do, in no particular order, to adopt this model more completely would be:
All of this can be done incrementally, and that's a good thing. :) Moving the handling of the "what do I present" logic into the exceptions, provides a clear place for that code and also creates a clearer boundary between the "core logic" vs "presentation logic". And, as we adopt this more-and-more, we'd also be creating a bigger gap between pip-crashed vs pip-presented-an-error -- which is also a good thing. :) |
We can do better than that, when there's no setuptools installed in the global environment and building an sdist via the legacy build system interface fails. |
I am quite impressed that it was merged, i really liked the PR! |
Current state of affairs here is:
|
As mentioned, I learnt a bunch of things about error message presentation while working on https://github.com/pradyunsg/sphinx-theme-builder. Transferring some of that presentation and implementation insights into this codebase in #10703. |
Another update:
|
Another situation for this -- poorly formatted dependencies on a package during development results in a horrible looking exception.
|
|
As outlined in #11816 - a trace message should not be thrown without explanation what the circumstances are. E.g. the version number of the tools pip and python, the environment being detected OS/venv and key settings like user or global install should be shown with general hints where to find help for the problem e.g. home page of the tools issue list and docs. It's also o.k. to do this in full detail only if a certain flag/environment variable is set and point this out in a shorter message which is the standard. |
I'm not sure about the current state of the issue. Apologies to anyone that's not interested 🙃 I'll mention my recent experience with Now there's:
That looks amazing 😍 Now, just a fewww improvements on that, and it'd be perfect (and hopefully not require
My scenario here goes that I have 0 options, so pip can say that. Or, it can say those are your valid options. It could be done so that some of those stages could be hidden ( |
@stdedos I'm pretty sure resolvelib doesn't provide an interface for changing the order of checks in the depresolver. |
24.1 (2024-06-20) ================= Vendored Libraries ------------------ - Upgrade truststore to 0.9.1. 24.1b2 (2024-06-12) =================== Features -------- - Report informative messages about invalid requirements. (`#12713 <https://github.com/pypa/pip/issues/12713>`_) Bug Fixes --------- - Eagerly import the self version check logic to avoid crashes while upgrading or downgrading pip at the same time. (`#12675 <https://github.com/pypa/pip/issues/12675>`_) - Accommodate for mismatches between different sources of truth for extra names, for packages generated by ``setuptools``. (`#12688 <https://github.com/pypa/pip/issues/12688>`_) - Accommodate for development versions of CPython ending in ``+`` in the version string. (`#12691 <https://github.com/pypa/pip/issues/12691>`_) Vendored Libraries ------------------ - Upgrade packaging to 24.1 - Upgrade requests to 2.32.0 - Remove vendored colorama - Remove vendored six - Remove vendored webencodings - Remove vendored charset_normalizer ``requests`` provides optional character detection support on some APIs when processing ambiguous bytes. This isn't relevant for pip to function and we're able to remove it due to recent upstream changes. 24.1b1 (2024-05-06) =================== Deprecations and Removals ------------------------- - Drop support for EOL Python 3.7. (`#11934 <https://github.com/pypa/pip/issues/11934>`_) - Remove support for legacy versions and dependency specifiers. Packages with non standard-compliant versions or dependency specifiers are now ignored by the resolver. Already installed packages with non standard-compliant versions or dependency specifiers must be uninstalled before upgrading them. (`#12063 <https://github.com/pypa/pip/issues/12063>`_) Features -------- - Improve performance of resolution of large dependency trees, with more caching. (`#12453 <https://github.com/pypa/pip/issues/12453>`_) - Further improve resolution performance of large dependency trees, by caching hash calculations. (`#12657 <https://github.com/pypa/pip/issues/12657>`_) - Reduce startup time of commands (e.g. show, freeze) that do not access the network by 15-30%. (`#4768 <https://github.com/pypa/pip/issues/4768>`_) - Reword and improve presentation of uninstallation errors. (`#10421 <https://github.com/pypa/pip/issues/10421>`_) - Add a 'raw' progress_bar type for simple and parsable download progress reports (`#11508 <https://github.com/pypa/pip/issues/11508>`_) - ``pip list`` no longer performs the pip version check unless ``--outdated`` or ``--uptodate`` is given. (`#11677 <https://github.com/pypa/pip/issues/11677>`_) - Use the ``data_filter`` when extracting tarballs, if it's available. (`#12111 <https://github.com/pypa/pip/issues/12111>`_) - Display the Project-URL value under key "Home-page" in ``pip show`` when the Home-Page metadata field is not set. The Project-URL key detection is case-insensitive, and ignores any dashes and underscores. (`#11221 <https://github.com/pypa/pip/issues/11221>`_) Bug Fixes --------- - Ensure ``-vv`` gets passed to any ``pip install`` build environment subprocesses. (`#12577 <https://github.com/pypa/pip/issues/12577>`_) - Deduplicate entries in the ``Requires`` field of ``pip show``. (`#12165 <https://github.com/pypa/pip/issues/12165>`_) - Fix error on checkout for subversion and bazaar with verbose mode on. (`#11050 <https://github.com/pypa/pip/issues/11050>`_) - Fix exception with completions when COMP_CWORD is not set (`#12401 <https://github.com/pypa/pip/issues/12401>`_) - Fix intermittent "cannot locate t64.exe" errors when upgrading pip. (`#12666 <https://github.com/pypa/pip/issues/12666>`_) - Remove duplication in invalid wheel error message (`#12579 <https://github.com/pypa/pip/issues/12579>`_) - Remove the incorrect pip3.x console entrypoint from the pip wheel. This console script continues to be generated by pip when it installs itself. (`#12536 <https://github.com/pypa/pip/issues/12536>`_) - Gracefully skip VCS detection in pip freeze when PATH points to a non-directory path. (`#12567 <https://github.com/pypa/pip/issues/12567>`_) - Make the ``--proxy`` parameter take precedence over environment variables. (`#10685 <https://github.com/pypa/pip/issues/10685>`_) Vendored Libraries ------------------ - Add charset-normalizer 3.3.2 - Remove chardet - Remove pyparsing - Upgrade CacheControl to 0.14.0 - Upgrade certifi to 2024.2.2 - Upgrade distro to 1.9.0 - Upgrade idna to 3.7 - Upgrade msgpack to 1.0.8 - Upgrade packaging to 24.0 - Upgrade platformdirs to 4.2.1 - Upgrade pygments to 2.17.2 - Upgrade rich to 13.7.1 - Upgrade setuptools to 69.5.1 - Upgrade tenacity to 8.2.3 - Upgrade typing_extensions to 4.11.0 - Upgrade urllib3 to 1.26.18 Improved Documentation ---------------------- - Document UX research done on pip. (`#10745 <https://github.com/pypa/pip/issues/10745>`_) - Fix the direct usage of zipapp showing up as ``python -m pip.pyz`` rather than ``./pip.pyz`` / ``.\pip.pyz`` (`#12043 <https://github.com/pypa/pip/issues/12043>`_) - Add a warning explaining that the snippet in "Fallback behavior" is not a valid ``pyproject.toml`` snippet for projects, and link to setuptools documentation instead. (`#12122 <https://github.com/pypa/pip/issues/12122>`_) - The Python Support Policy has been updated. (`#12529 <https://github.com/pypa/pip/issues/12529>`_) - Document the environment variables that correspond with CLI options. (`#12576 <https://github.com/pypa/pip/issues/12576>`_) - Update architecture documentation for command line interface. (`#6831 <https://github.com/pypa/pip/issues/6831>`_) Process ------- - Remove ``setup.py`` since all the pip project metadata is now declared in ``pyproject.toml``. - Move remaining pip development tools configurations to ``pyproject.toml``.
What's the problem this feature will solve?
pip's error messages do not provide any context on what component or thing is involved in the problem, and where the bug/issue might lie.
Describe the solution you'd like
Add clear diagnostic information about the $thing that likely caused the error (eg: package, network, index page). We could also add hints toward potential solutions, in cases where we know the likely causes (eg: this is an issue with <url>, or this package is not supported on your platform, or did you setup an environment to build the package).
Alternative Solutions
Maintain status quo? We've got adhoc messages right now, which may or may not contain diagnostic information for the user, usually not.
Additional context
This is a pattern of behaviour I've seen on the issue tracker, in conversational feedback about pip's error messages, and seems to align with my personal effort of "what can we learn from other package managers to improve".
The text was updated successfully, but these errors were encountered: