openownership / data-standard-sphinx-theme

Documentation theme for the Beneficial Ownership Data Standard.
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

sphinxcontrib-jsonschema extension is not working #4

Open odscjames opened 6 years ago

odscjames commented 6 years ago

maximum 1 argument(s) allowed, 3 supplied.

When it is working, all csv-table directives need to be changed to this.

odscjames commented 6 years ago

@bjwebb Can I ask if you have any pointers on this?

I installed sphinxcontrib-jsonschema via pip and get:

/vagrant/data-standard/docs/schema.md:1: ERROR: Error in "jsonschema" directive:
maximum 1 argument(s) allowed, 3 supplied.

.. jsonschema:: ../schema/beneficial-ownership-statement.json
    :collapse: statementGroups/0/beneficialOwnershipStatements,statementGroups/0/entityStatements,statementGroups/0/personStatements
/vagrant/data-standard/docs/schema.md:1: ERROR: Error in "jsonschema" directive:
maximum 1 argument(s) allowed, 5 supplied.

.. jsonschema:: ../schema/beneficial-ownership-statement.json
    :pointer: /definitions/BeneficialOwnershipStatement
    :collapse: interests
odscjames commented 6 years ago

My notes: https://github.com/openownership/data-standard/commit/76a9eb49534d148db1a02447d3b011548077e493 is the commit where these directives was introduced but I can't see anything about including libraries here.

Bjwebb commented 6 years ago

We have a custom version of sphinxcontrib-jsonschema, have you got that installed? https://github.com/openownership/data-standard/blob/60-refactor-docs/requirements.txt#L7

odscjames commented 6 years ago

Nope, I was using pip! Thanks, I will check that out.

Bjwebb commented 6 years ago

BTW, that file can be installed with pip with pip install -r requirements.txt. Just this dependency could be installed with pip install -e git+https://github.com/OpenDataServices/sphinxcontrib-jsonschema.git@e39a92f4238bf022db11e4e88ba92fdaff2b31c9#egg=sphinxcontrib-jsonschema

Bjwebb commented 6 years ago

The instructions at https://github.com/openownership/data-standard/tree/60-refactor-docs#build-the-docs-locally might also be useful for getting set up.

odscjames commented 6 years ago

Fixed that error by installing lib directly. (I should maybe look at using requirements.txt for this project generally.)

I now have another error:

jsonref.JsonRefError: ("Unresolvable JSON pointer: '/definitions/ID'", OrderedDict([('$ref', 'components.json#/definitions/ID')]))

That does seem to be in the file correctly. I will try and work out what is going on in a day or two.

odscjames commented 6 years ago

This is broken because!

When @Bjwebb added the directives the data files did not have any external references Then Jack added some external references And our own library does not support external refs: https://github.com/OpenDataServices/sphinxcontrib-jsonschema/blob/master/sphinxcontrib/jsonschema.py#L30

odscjames commented 6 years ago

Ok, so this commit https://github.com/OpenDataServices/sphinxcontrib-jsonschema/commit/b7332e8fd5c788ee59b983396102ddc38967a3d0 adds the placeholder class that doesn't allow any loading. If I reverse the commit, it will now .... let us move on to the next problem! :tada:

I don't think the https://pypi.org/project/jsonref/ library supports external refs the way we use them. See https://github.com/gazpachoking/jsonref/blob/master/jsonref.py#L295 If it's http/https and a requests lib is installed, load using requests! That's great. If it's not -- load using urllib.request.urlopen ... which also expects a URI. In other words, just passing "components.json" does not work!

If you go through the source file and replace any "components.json" with "https://raw.githubusercontent.com/openownership/data-standard/60-refactor-docs/schema/components.json" or "file:///vagrant/data-standard/schema/components.json" that bypasses this issue! :tada:

And we move on to .... another issue! :cry: I haven't yet looked at that issue properly yet, I'm just noting this issue before I move on so I don't forget it all and run away and join the circus. :clown_face:

odscjames commented 6 years ago

Ok, final problem is simply that i think

eval_rst
.. jsonschema:: ../schema/beneficial-ownership-statement.json
    :pointer: /definitions/BeneficialOwnershipStatement
    :collapse: interests

has now been broken by the splitting into separate JSON files - beneficial-ownership-statement.json has no definitions object.

So, to list the things that I think need fixing:

and then finally,

Yup, the circus definitely looks tempting :circus_tent: :stuck_out_tongue:

odscjames commented 6 years ago

https://pypi.org/project/jsonref/ does not support $ref's of just a file name

Oh wait, it does! But you have pass base_uri! https://github.com/gazpachoking/jsonref/issues/8#issuecomment-226969318

odscjames commented 6 years ago

So progress has been made on this!

https://github.com/OpenDataServices/sphinxcontrib-jsonschema/pull/8 has been opened to discuss the changes needed there.

About to do a commit so that the vagrant box will use the version of the library that is in the P.R.

Then, what is left to do: