Add support for constructed nested schemas #18

[mgd020]

Also isort imports in base.py and ignored .pytest_cache

Fix for #18

[coveralls]

Coverage remained the same at 100.0% when pulling 3c606659f1f8261e8b28c5e468a12ad0714c0b45 on mgd020:issue-18 into f6c2daecdab2580fb37eb7bf02b3eefd8176de1d on fuhrysteve:master.

[atmo]

Hi @fuhrysteve @mgd020 , could you please clarify what's the status of this PR? I wouldn't mind implementing any of the corrections needed myself to get this into master.

[mgd020]

@atmo Not a question for me. Its ready to merge

[atmo]

@mgd020

1. You'd still need to resolve conflicts.
2. I still would like to see the issue I've mentioned in the comment above resolved. That is, I don't see any point in creating a new schema object if you have one that's perfectly fine to pass around.

[mgd020]

I was expecting the PR to show any merge conflicts?

Also, cant see any comments above your one this morning.

[mgd020]

Looking at the PR changes it does not create a new object?

[atmo]

@mgd020 but there is one. Have a look at marshmallow_jsonschema/base.py, line 190. You're getting a class from a schema instance in case this object is indeed an instance of SchemaABC. Then you're using this class to instantiate a schema on the line 205. However, my argument is that you don't need to do that in case field.nested is already an instance. Just check whether field.nested is a class and a subclass of Schema and if so, use it to instantiate an object. Otherwise, do nothing.

Here is a sample code how it might look like:

from inspect import isclass
        ...
        if name not in self._nested_schema_classes and name != outer_name:
            wrapped_nested = JSONSchema(nested=True)
            if isclass(nested) and issubclass(nested, SchemaABC):
                nested = nested(only=only, exclude=exclude, context=context)
             wrapped_dumped = wrapped_nested.dump(nested)
         ...

For this to work you'll have to also get rid of elif isinstance(field.nested, SchemaABC): check on line 189.

Does it make sense?

[mgd020]

I was using mobile view so could not see the "conflicts". Also, I expect there to be conflicts after almost a year!

I have not looked at the code recently, but I would guess that it still needs to create a new instance as only and exclude are passed in.

I am happy for you to take over this PR / fix, as I no longer have time to fix this.

[atmo]

@mgd020 Sure, no problem. @fuhrysteve , please close this PR in favor of #74 and let's continue discussion there.