jacksmith15 / statham-schema

Statham is a Python Model Parsing Library for JSON Schema.

https://statham-schema.readthedocs.io/en/latest/

MIT License

38 stars 11 forks source link

Use description field for doc comments. #93

Closed SolidTux closed 1 year ago

SolidTux commented 1 year ago

Use description field to generate doc comments

Check list

Before asking for a review

[x] Target branch is master
[x] make test passes locally. ~(many don't pass, but they also don't without my changes)~
[x] [Unreleased] section in CHANGELOG.md is updated.
[x] I reviewed the "Files changed" tab and self-annotated anything unexpected.

Before review (reviewer)

[ ] All of the above are checked and true. Review ALL items. If not, return the PR to the author.
[ ] Continuous Integration is passing. If not, return the PR to the author.

After merge (reviewer)

[ ] Any issues that may now be closed are closed.

SolidTux commented 1 year ago

This is a proof of concept to use the description field of properties to generate doc comments when using the python serializer.

Is this a good idea? In general it would be great to have some way of generating doc comments and the description seems appropriate. But especially with object properties this is a bit weird as it uses the same string for the documentation of the property and the class.

jacksmith15 commented 1 year ago

@SolidTux Thanks for submitting, this is definitely desirable. I'll aim to review in more depth today.

jacksmith15 commented 1 year ago

@SolidTux as an aside, I'd be interested if you could share your output of make test on master.

If the error relates to a missing file, please ensure you have followed the development set-up instructions, in particular:

git submodule update --init --recursive

jacksmith15 commented 1 year ago

@SolidTux Thanks again for submitting. I've proposed a few changes to the intended behaviour, but implementation otherwise looks good.

test_that_main_produces_expected_output fails on a few cases where the schema under test contains descriptions. This test just runs code generation on each of the schemas in tests/jsonschemas/ and compares the resulting ASTs to the corresponding files in tests/models/.

Resolving these should be as simple as updating the files in tests/models/ to include the appropriate docstrings and description fields.

SolidTux commented 1 year ago

Thank you a lot for the fast review and answer! I will have a look after the weekend.