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

Add support for metadata on custom fields. #21

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

Conversation

eligundry
Copy link
Contributor

Custom fields now can have titles and stuff!

@coveralls
Copy link

coveralls commented Nov 30, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling ff6bf10 on eligundry:improve-custom-types into 6f51771 on fuhrysteve:master.

@fuhrysteve
Copy link
Owner

fuhrysteve commented Jan 8, 2017

Sorry I didn't notice this before.

I like this functionality - I think even better than the _jsonschema_type_mapping hack I had. I think its only weakness is that it depends on that function to be there for this to work.

For example, this would not work:

from marshmallow import fields, Schema

class MySchema(Schema):
    name = fields.String(metadata={"title": "Full Name"})

What if we dropped the whole _jsonschema_type_mapping concept and instead make the API something like this:

from marshmallow import fields, Schema

class MySchema(Schema):
    name = fields.String(jsonschema_attributes={"title": "Full Name"})

When I originally made the _jsonschema_type_mapping method, I wasn't fully aware of the **metadata on fields. We whould be able to access it by doing something like:

field.metadata.get('metadata', {}).get('jsonschema_attributes')

Let me know what you think or if I missed anything!

@Lacrymology
Copy link

bump. I really like this, and I think I'd need it to be able to integrate this library into my project

@atmo
Copy link
Contributor

atmo commented Jul 10, 2019

Hi @fuhrysteve @eligundry , 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.

@fuhrysteve
Copy link
Owner

If I'm not mistaken, you should be able to get this functionality by implementing _jsonschema_type_mapping today - however if you have ideas for how to improve that pattern circa this pull request, I'm not opposed to it!

To help me understand what we're working towards here, would you mind showing an example of what the API change would be (from a high level) so it's clear why this is an improvement on what already exists today?

@fuhrysteve
Copy link
Owner

Is this just a way to make it so that you don't have to implement your own type in order to inject stuff into properties?

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 this pull request may close these issues.

None yet

5 participants