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 fields from APIFlask? #280

Closed
sloria opened this issue Jan 12, 2024 · 8 comments
Closed

Add fields from APIFlask? #280

sloria opened this issue Jan 12, 2024 · 8 comments

Comments

@sloria
Copy link
Member

sloria commented Jan 12, 2024

apiflask has File and Config fields, which may be appropriate to have in flask-marshmallow: https://github.com/apiflask/apiflask/blob/main/src/apiflask/fields.py

as well as validators to use with File fields: https://github.com/apiflask/apiflask/blob/main/src/apiflask/validators.py

@sloria
Copy link
Member Author

sloria commented Jan 12, 2024

cc @greyli , you may be interested in this :)

@greyli
Copy link
Contributor

greyli commented Jan 13, 2024

Sure, I could help migrate them here. I thought about it, but it was lost on my to-do list.

I will submit a pull request to add the Config field.

@uncle-lv Could you help migrate the File related field and validators to flask-marshmallow?

@uncle-lv
Copy link
Contributor

Sure, I could help migrate them here. I thought about it, but it was lost on my to-do list.

I will submit a pull request to add the Config field.

@uncle-lv Could you help migrate the File related field and validators to flask-marshmallow?

Of course.

@sirosen
Copy link
Collaborator

sirosen commented Jan 13, 2024

I haven't done very much here since I first got involved on this project, but I'd be happy to help with these too!

Config seems pretty small and easy to grasp. I'll need to read up a bit on apiflask's current File tooling.

@greyli
Copy link
Contributor

greyli commented Jan 14, 2024

Created the pull request for the Config field: #281

@sirosen
Copy link
Collaborator

sirosen commented Jan 16, 2024

Wow, these got done in a flash! I think this is good to close, but please reopen if I'm wrong.

I noticed that Config uses _CHECK_ATTRIBUTE, which feels a little like an implementation detail in marshmallow leaking into downstream libraries. Would it be better for the field to define serialize instead of _serialize? (I haven't thought about this very hard, so apologies if that is just wrong-headed. 😄 )

@sirosen sirosen closed this as completed Jan 16, 2024
@sloria
Copy link
Member Author

sloria commented Jan 16, 2024

_CHECK_ATTRIBUTE isn't just an implementation detail--it's meant to be set by subclasses of Field when the attribute isn't meant to be accessed on the object to be serialized: https://github.com/marshmallow-code/marshmallow/blob/e099e058f8d17af08369d4485622d1c14f9084ed/src/marshmallow/fields.py#L138-L141

Config is very similar to fields.Constant, which sets _CHECK_ATTRIBUTE to False.

@sloria
Copy link
Member Author

sloria commented Jan 16, 2024

These changes are released in 1.0.0

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

No branches or pull requests

4 participants