Closed cblegare closed 3 years ago
Thanks for submitting this -- I did actually think about adding this sort of validation to the DataObj
classes, early on.
However, I don't think Sphinx uses priority
when constructing cross-references, especially via intersphinx
-- as best I can tell, the value is only used by Sphinx in the process of building docs, and by the time it's exported into objects.inv
, it's not actually used further. So, I decided against any particular validation on it, since there may be use cases where arbitrary priority
values might be desirable, for purposes other than just use with intersphinx
.
But: Are you seeing a particular bug as a result of this? If there is a functional reason why priority
should be constrained, then definitely I'd want to add validation for it.
Thank you for your feedback!
I indeed see a bug since the sphobjinv.Inventory
could not import my objects.inv
file. If you choose to allow arbitrary priorities, you might want to change the regex of sphobjinv.re.ptn_data
to allow it too ;)
Ahh, indeed, I've restricted it to integers, haven't I!
Can you supply me with your objects.inv
, so that I can use it for testing?
Needs:
priority
values to docs, and motivation for making priority
generalpriority
values~priority
values~hypothesis
for this~objects.inv
with 0.5
priority values should suffice, mostlypriority
, including the sphinx_load_test
Thank you for reporting this!
Yes sure! I'll upload it here for now, but it might be available online quite soon
Here is a plain text version of it
Ah, that's a working version of the mdn inventory... would it be possible for you to generate a version that soi.Inventory
can't load, but that you would like it to be able to? (E.g., with the 0.5
values for priority
you mentioned.)
Yes sorry about that
For documentation purposes: Sphinx does only use the priority
(prio
within Sphinx) string value as a numeric value, in both the Python and JS search code.
However, these uses are only relevant to HTML documentation built by Sphinx—intersphinx
doesn't use the priority
value at all—and so it seems like there's little reason to constrain objects.inv
files created de novo to numerical priority
values; it should be fine for them to be any non-empty string value containing no whitespace.
I'll try to get a prerelease of this fix up onto PyPI sometime today (US Eastern).
Done. Give it a shot with pip install --pre
, let me know how it goes.
Per https://github.com/bskinn/sphobjinv/issues/181#issuecomment-773323476, the relaxation of accepted priority
values is incompatible with the regex Sphinx uses when loading objects.inv
files. This will lead to the object entries being silently ignored by Sphinx, and thus in turn lead to broken cross-references.
I'm going to have to revert the priority
change for v2.1. Rigorous validation (#152) is a breaking change, so that won't happen until a v3.0 release. Don't know what the timeline will be for that yet... probably not any time soon.
While generating a objects.inv from Mozilla MDN reference, I reused their "priority" settings to generate
DataObjStr
objects (usually0.5
), which is invalid as per https://sphobjinv.readthedocs.io/en/latest/syntax.html?highlight=priority#sphinx-objects-inv-v2-syntaxThere is no such validation in the data model, and that could be a nice addition :)