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

Bug in v1.16.4 when lift_title=False broke auto_definitions #50

Closed
ankostis opened this issue Oct 4, 2020 · 11 comments · Fixed by #51
Closed

Bug in v1.16.4 when lift_title=False broke auto_definitions #50

ankostis opened this issue Oct 4, 2020 · 11 comments · Fixed by #51
Assignees

Comments

@ankostis
Copy link

ankostis commented Oct 4, 2020

With latest v1.16.4 and config-options:
"lift_title": False,
"lift_definitions": True,
"auto_reference": True,
"auto_target": True,

... then the auto_definitions=True does not work -- all definition references are reported as

WARNING: Unknown target name: "<schema-name>"

Turning lift_title=True fixes the problem, but then one cannot take advantage of this setting.

@lnoor
Copy link
Owner

lnoor commented Oct 4, 2020

Not so sure it is lift_definitions per se.
The error is a reference error.
It rather feels like there is an issue in combination with auto_reference or perhaps auto_target.

The thing is that to have a working reference you need to have a title using lift_title = True.
There is no way around that.

Could you please try the different combinations?
All options False should work, then set lift_definitions True and so on.

@lnoor lnoor self-assigned this Oct 4, 2020
@ankostis
Copy link
Author

ankostis commented Oct 4, 2020

Assuming a schema with definitions and directives referencing with json-pointer, and without thoroughly digging into each case below, this is the situation:

  • When auto_xxx=True AND lift_references=True AND lift_title=FALSE, everything works fine only (lift_description is irrelevant).
  • auto_xxx=True AND lift_references=True AND lift_title=True, fails with warnings(!) for schemas with title elements, but definitions work fine.
  • All false, fail with _warnings(!) for schemas with references_again, and definitions expectedly do not work.
  • The same failures happen when just auto_xxx=True OR just lift_references=True.

Do that make sense? Or i should do elaborate testing?

@lnoor
Copy link
Owner

lnoor commented Oct 4, 2020

That is kind of what I expected except the second case, I would expect that one to work.
I thought at first it was a simple misunderstanding.
I will create my own test sets and experiment to understand better what is happening.
It will be a couple of days before I can look into it.

@lnoor
Copy link
Owner

lnoor commented Oct 7, 2020

Given this schema:

.. jsonschema::
    :lift_title: True
    :lift_definitions: False
    :auto_target: False
    :auto_reference: False

    {
        "title": "outermost title",
        "description": "outermost description",
        "type": "object",
        "properties": {
            "param1": {"$ref": "#/definitions/type1"},
            "param2": {"$ref": "#/definitions/type2"}
        },
        "required": ["param1"],
        "definitions": {
            "type1": {
                "$$target": "#/definitions/type1",
                "title": "type1 title",
                "description": "type1 description",
                "type": "integer",
                "minimum": 10,
                "maximum": 50
            },
            "type2": {
                "$$target": "#/definitions/type2",
                "title": "type2 title",
                "description": "type2 description",
                "type": "string",
                "minLength": 8
            }
        }
    }

Tests and results:

I suppose your issue is with cases 9 and 10. Is that correct?

I would have thought that the combination of auto_target and auto_reference would have worked.
From the table it appears that auto_target makes no difference.

The (surprising) case 4 suggests a temporary solution for you.

case lift_title lift_def auto_tar auto_ref comment make output page displays
1 True False False False the default situation WARNING: undefined label: #/definitions/type1 (if the link has no caption the label must precede a section header) WARNING: undefined label: #/definitions/type2 (if the link has no caption the label must precede a section header) single table preceded by title value for param1: #/definitions/type1 value for param2: #/definitions/type2
2 False False False False same as above same as above
3 True True False False no error three tables each preceded by its title value for param1: proper link to type1 value for param2: proper link to type2
4 False True False False Unexpected result no error same as above
5 True True False False Removed both $$target entries on all remaining cases WARNING: undefined label: #/definitions/type1 (if the link has no caption the label must precede a section header) WARNING: undefined label: #/definitions/type2 (if the link has no caption the label must precede a section header) three tables each preceded by its title value for param1: #/definitions/type1 value for param2: #/definitions/type2
6 True True True False as above as above
7 True True False True no error three tables each preceded by its title value for param1: proper link to type1 value for param2: proper link to type2
8 True True True True as above as above
9 False True False True values look like links but do not function properly WARNING: Unknown target name: "type1 title". WARNING: Unknown target name: "type2 title". value for param1: type1 title value for param2: type2 title
10 False True True True as above as above as above

@ankostis
Copy link
Author

ankostis commented Oct 7, 2020

Thanks.
I thought of trying to fiddle with the "description", but figured it wouldn't make a difference.
Now i can think of an explanation: it needs at least a proper text element for the target anchor to attach to.

@WouterTuinstra
Copy link

I have found the cause the new introduced option :lift_title: is causing issues with only :auto_reference:.
The :auto_reference: option used titles to create link to that part of the table.
The :auto_reference: worked without using option :auto_target:
The fix would be to update :auto_reference: and use the created targets by :auto_target: if :lift_title: False

WouterTuinstra pushed a commit to WouterTuinstra/sphinx-jsonschema that referenced this issue Oct 7, 2020
@ankostis
Copy link
Author

ankostis commented Oct 8, 2020

I noticed that while lift_description workarounds the bug, the following setup does not "lift" the descriptions, and these are also embedded in the tables:

jsonschema_options = {
    "lift_title": True,
    "lift_description": True,
    "lift_definitions": True,
    "auto_reference": True,
    "auto_target": True,
}

image

@ankostis
Copy link
Author

ankostis commented Oct 8, 2020

(ok, my last message meshed it up)
As you can see, my configuration had actually lift_title: True (despite my claims in my comment) and that is why the bug did not appear.
When i set lift_title: False (like below), the `WARNING Unknown target name" for all definitions are raised.

jsonschema_options = {
    "lift_title": False,
    "lift_description": True,
    "lift_definitions": True,
    "auto_reference": True,
    "auto_target": True,
}

And i confirm that if all the rest are true, auto_target: False does not cause problems.


To be succinct, the workaround no4 does not work when schema reuses definitions.

@WouterTuinstra
Copy link

To prevent messing it up any further but quickly answer your earlier message
The lift_description was designed to work only with the combination of a title see also documentation.
Maybe discuss separately if required.

@WouterTuinstra
Copy link

WouterTuinstra commented Oct 8, 2020

It's a bit annoying how docutils reports broken links but the WARNING Unknown target name it's generated if there is a reference created but no target to link to.

Regarding case 4 the schema used has a $$target that's equal to the definition if :auto_reference: False it uses the old method that's why it works.

@lnoor
Copy link
Owner

lnoor commented Oct 11, 2020

case 4: except that lift_title was False. That was the surprising bit for me.

@lnoor lnoor closed this as completed in #51 Oct 11, 2020
lnoor added a commit that referenced this issue Oct 11, 2020
Fixes #50 auto reference without title
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

Successfully merging a pull request may close this issue.

3 participants