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

intersphinx: Reduce log message severity when ambiguous target resolves to a duplicate #12587

Conversation

jayaddison
Copy link
Contributor

@jayaddison jayaddison commented Jul 15, 2024

Feature or Bugfix

  • Bugfix

Purpose

  • Don't emit warnings about ambiguous entries that are pure-duplicates during resolution of Intersphinx references.

Detail

  • Use set-comparison of the reference fields. however, do use a set to count the number of distinct matched definitions at resolution-time.
  • Do log a debug messages for reference.

Relates

Edit: redraft description since #12586 was merged separately.
Edit: remove resolves-issue reference.

…on' into issue-12585-extra/objectsinv-resolution-ambiguity-false-positive-reduction
…biguity-false-positive-reduction

Conflicts:
	sphinx/util/inventory.py
@jayaddison jayaddison changed the title intersphinx: Don't warn about pure-duplicate ambiguous definitions during either loading or resolution. intersphinx: Don't warn about ambiguous target resolution in the case of duplicates. Jul 15, 2024
@jayaddison jayaddison changed the title intersphinx: Don't warn about ambiguous target resolution in the case of duplicates. intersphinx: Reduce log message severity when ambiguous target resolves to a duplicate. Jul 15, 2024
@jayaddison
Copy link
Contributor Author

@AA-Turner a note to let you know that I'm holding this change in reserve - it is about Intersphinx resolution -- e.g. occurs when people attempt to include links to potential ambiguity in external projects from their own.

This should happen far less often than ambiguity-detection at inventory loading-time, and I think is a more serious scenario (because it could result in incorrect hyperlinks). Even so, the pure-duplicate case might make sense to reduce log-severity for here too.

@jayaddison
Copy link
Contributor Author

(I'm not sure that I explained that particularly clearly, so let me know if I can clarify. there are a few similar-seeming changes in-flight here, and I regret that because I could have planned this rollout better to reduce the chance of confusion)

…biguity-false-positive-reduction

Conflicts:
	CHANGES.rst
@jayaddison jayaddison marked this pull request as ready for review July 18, 2024 15:40
@jayaddison
Copy link
Contributor Author

Following the principle of "who is this warning actionable for" (ref here), I don't think that either the referrer or the referee are likely to want or need to do anything about ambiguities that are effectively duplicate entries. It's potentially useful to be aware of these cases, but not something that should block strict-mode builds.

@jayaddison jayaddison requested a review from chrisjsewell July 18, 2024 15:48
@jayaddison

This comment was marked as resolved.

…biguity-false-positive-reduction

Conflicts:
	CHANGES.rst (manual adjustment)
@jayaddison

This comment was marked as resolved.

@AA-Turner
Copy link
Member

Does this address a regression in 7.4? Otherwise I will hold off until 7.5 // 8.0.

A

@jayaddison
Copy link
Contributor Author

Does this address a regression in 7.4? Otherwise I will hold off until 7.5 // 8.0.

It does not, no - and that sounds fine to me, thank you 👍

@jayaddison jayaddison changed the title intersphinx: Reduce log message severity when ambiguous target resolves to a duplicate. intersphinx: Reduce log message severity when ambiguous target resolves to a duplicate Jul 23, 2024
…biguity-false-positive-reduction

Conflicts:
	CHANGES.rst
	tests/test_extensions/test_ext_intersphinx.py
@AA-Turner AA-Turner merged commit 655d1c7 into sphinx-doc:master Aug 11, 2024
20 checks passed
@jayaddison jayaddison deleted the issue-12585-extra/objectsinv-resolution-ambiguity-false-positive-reduction branch August 11, 2024 16:57
@jayaddison
Copy link
Contributor Author

Thanks again!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 11, 2024
@AA-Turner AA-Turner added this to the 8.1.x milestone Oct 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants