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

Improve error messages #9

Open
tdejager opened this issue Dec 15, 2023 · 6 comments
Open

Improve error messages #9

tdejager opened this issue Dec 15, 2023 · 6 comments

Comments

@tdejager
Copy link
Contributor

Currently it is possible to add your own custom error messages. However, it can now quickly explode with the information that is displayed.

E.g with usage in RIP

  × Could not solve for the requested requirements:
  │ The following packages are incompatible
  │ |-- opentelemetry-exporter-prometheus * cannot be installed because there are no viable options:
  │ 	|-- opentelemetry-exporter-prometheus 1.12.0rc1 is excluded because prereleases are not allowed
  │ 	|-- opentelemetry-exporter-prometheus 1.10a0 is excluded because prereleases are not allowed
  │ 	|-- opentelemetry-exporter-prometheus 0.42b0 is excluded because prereleases are not allowed
  │ 	|-- opentelemetry-exporter-prometheus 0.41b0 is excluded because prereleases are not allowed
  │ 	|-- opentelemetry-exporter-prometheus 0.40b0 is excluded because prereleases are not allowed
  │ 	|-- opentelemetry-exporter-prometheus 0.39b0 is excluded because prereleases are not allowed
  │ 	|-- opentelemetry-exporter-prometheus 0.38b0 is excluded because prereleases are not allowed
  │ 	|-- opentelemetry-exporter-prometheus 0.37b0 is excluded because prereleases are not allowed
  │ 	|-- opentelemetry-exporter-prometheus 0.36b0 is excluded because prereleases are not allowed
  │ 	|-- opentelemetry-exporter-prometheus 0.35b0 is excluded because prereleases are not allowed
  │ 	|-- opentelemetry-exporter-prometheus 0.34b0 is excluded because prereleases are not allowed
  │ 	|-- opentelemetry-exporter-prometheus 0.33b0 is excluded because prereleases are not allowed
  │ 	|-- opentelemetry-exporter-prometheus 0.32b0 is excluded because prereleases are not allowed
  │ 	|-- opentelemetry-exporter-prometheus 0.30b1 is excluded because prereleases are not allowed
  │ 	|-- opentelemetry-exporter-prometheus 0.30b0 is excluded because prereleases are not allowed
  │ 	|-- opentelemetry-exporter-prometheus 0.29b0 is excluded because prereleases are not allowed
  │ 	|-- opentelemetry-exporter-prometheus 0.17b0 is excluded because prereleases are not allowed
  │ 	|-- opentelemetry-exporter-prometheus 0.16b1 is excluded because prereleases are not allowed
  │ 	|-- opentelemetry-exporter-prometheus 0.16b0 is excluded because prereleases are not allowed
  │ 	|-- opentelemetry-exporter-prometheus 0.15b0 is excluded because prereleases are not allowed
  │ 	|-- opentelemetry-exporter-prometheus 0.14b0 is excluded because prereleases are not allowed
  │ 	|-- opentelemetry-exporter-prometheus 0.13b0 is excluded because prereleases are not allowed
  │ 	|-- opentelemetry-exporter-prometheus 0.12b0 is excluded because prereleases are not allowed

It would be good to create certain version ranges for instance that could collapse into a single message.

Also see this issue in rip for another example: prefix-dev/rip#26

The difficulty with this is that users can provider their own way to display errors so ideally we provide the tools to make these error messages nice. I think there are some ways to generalize this over packaging ecosystems.

@baszalmstra
Copy link
Contributor

baszalmstra commented Dec 15, 2023

See also: pubgrub-rs/pubgrub#163

Or more generally the work on the simplify method in pubgrub.

@tdejager tdejager changed the title Make error messages a bit better Improve error messages Dec 15, 2023
@aochagavia
Copy link
Contributor

aochagavia commented Jan 31, 2024

Note that we already implement something akin to PubGrub's error simplification algorithm, based on this blogpost.

There are still a few improvements we could make, though:

  1. Easy (edit: done in this PR): collapse installable versions into a single line, instead of displaying each of them in its own line (we are already doing this for incompatible packages, e.g. |-- requests 2.2.1 | 2.3.0 | 2.4.0 | 2.4.1 | ...).
  2. Hard: try to infer ranges based on the package versions, so we can display them instead. This would look like |-- requests >=2.2.1, <= 2.31.0. This is much more concise than listing ~20 different versions! The hard part is, however, that ranges are not necessarily contiguous so there might be edge cases to watch out for. I'll be thinking about it.

Edit: I also see now that the simplification step doesn't seem to be implemented for Excluded stuff. That's something to fix as well (sub-edit: done in this PR).

@wolfv
Copy link
Member

wolfv commented Jan 31, 2024

I think computing ranges shouldn't be that hard, should it? We would just need to:

  • take the minimum version of conflicting packages
  • find all solvables
  • walk upwards until we hit a non-conflicting package and that's the upper bound for the range that we can display to the user

But maybe I am not seeing some complications.

@aochagavia
Copy link
Contributor

Maybe it is easy after all, I'll be prototyping :)

@AntoinePrv
Copy link
Member

Version ranges would be nice indeed. We never looked into it in Mamba because when this was done we had no Version implementation (all deep inside libsolv) and were limited to treating it as strings.
Two complications might be:

  • complex conflicts where representing the set as [min, max] might include packages that were not in the list (I don't think this happens often).
  • Including build variant, e.g. for Pytorch CUDA vs CPU you'd not want to merge all that information. So you'd keep the build string, but they are all different. Perhaps we need to include some hand-tunes rule looking for things like "cuda", "cpu", "py39", "mpi", "blas" in the build strings. I started adding such rules to "clean" the python python_abi mess.

But definitely worth a try ! Another thing I would have tried was changing the merge criteria more or less aggressively, depending on if packages have the same version major or minor etc. (this would then be controlled by verbosity).

@Eh2406
Copy link

Eh2406 commented Jan 31, 2024

In PubGrub-rs we had a similar issue with converting 1 | 2 | 3 | ... | n - 1 | n into >=1 & <=n, while correctly handling important holes. The PR I cam up with is at pubgrub-rs/pubgrub#156. I think it can be used by constructing a range that is the union of singletons for all versions that have the same reason, then simplify the result.

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

No branches or pull requests

6 participants