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

Adding the ability to set a 'required' boolean kwarg in the Object.add_object method #60

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sj-curtin
Copy link

  • Doing so will avoid the required keyword for an object in
    the schema to be included
  • Added test cases
  • Updated docs

Added GenSON to a recent project and found it very useful - thank you 🙌

Use case:

"If JSON is validated against a schema produced and it is missing an object key, it should still be deemed valid"

Our solution was a little overwrite of the add_object method for Objects - however we also toyed with this approach - what are your thoughts?

Our change:

mock_dict = {"mock_value_one": "abc"}
builder = SchemaBuilder()
builder.add_object(mock_dict, required=False)
print(builder.to_json())

>>> {"$schema": "http://json-schema.org/schema#", "type": "object", "properties": {"mock_value_one": {"type": "string"}}}

Default:

mock_dict = {"mock_value_one": "abc"}
builder = SchemaBuilder()
builder.add_object(mock_dict)
print(builder.to_json())

>>> {"$schema": "http://json-schema.org/schema#", "type": "object", "properties": {"mock_value_one": {"type": "string"}}, "required": ["mock_value_one"]}

kwarg in the Object.add_object method

- Doing so will avoid the `required` keyword for an object in
the schema to be included
- Added test cases
- Updated docs
@wolverdude
Copy link
Owner

Thanks for the PR! Sorry for the slow response here. This looks really useful, but I'm still hesitant to approve because it doesn't really fit well with the existing philosophy of how GenSON works and there are a couple of edge-cases that need to be explored.

I think I would like to understand more about your use-case. I can see two possibilities here, and there are alternative approaches for each that are more consistent with GenSON's design:

  1. You want to include required in some object nodes but not others within the same schema.

    This can sort of be accomplished with the use of a seed schema, though for input-output consistency reasons, you will necessarily get "required": [] in the output schema. If you don't want that, I don't have a good solution for you, so I'm open to a PR, but it might make more sense for us to have an empty_required option on SchemaBuilder or something like that rather than using add_object().

    I should note that I don't really recommend using add_object() to selectively change the behavior of nodes (e.g. some nodes will ignore required and others won't). This is because it will become difficult to know which nodes are behaving which way if you don't know the full structure of the objects you are adding (I presume this is why you use GenSON in the first place 🙂). Using seed schemas eliminates this ambiguity and gives you deterministic control over node behavior.

    If you don't know anything about the structure of the object beforehand though, seed schemas are not an option. But then if you don't know anything about the structure, how are you supposed to know which nodes shouldn't have required and which should? I'm assuming that you don't have this requirement, but if you do, I'd be very curious to hear about your use-case.

  2. You want to create a schema with no required key anywhere.

    The easiest way to accomplish this is to create a custom SchemaBuilder with a custom Object strategy that inherits from the original Object strategy and deletes any required property after calling super in to_schema(). You can do that yourself in your own code without having to wait on my slow, opinionated butt to approve it.

I'm not sure I answered everything. Please let me know any further thoughts or context. I do have one more thought here: I notice that you added a **kwargs arg to add_object, which gives more opportunity for backwards-compatible future extension. I kinda like that. Mostly, I think it might be useful to people who want to create custom schema strategies. I would probably include a warning about the nondeterminism thing I mentioned above, but if people would find it useful, I would consider including it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants