Marshmallow: Schema class fields deletion control

[decaz]

I see at _get_fields function there is a pop parameter that defines what to do with field attributes of schema class:
https://github.com/marshmallow-code/marshmallow/blob/09c5b26bee77be5e23927ddeb472d5ea56733266/src/marshmallow/schema.py#L54-L56
There is also ordered parameter here but unlike him pop parameter isn't accessible through the API. I think it would be nice to have the corresponding meta parameter that control schema class fields deletion.

[lafrech]

Can you please elaborate about your use case?

It's not clear to my why someone would want to do that.

(BTW? I'm wondering about the field_class parameter. AFAIU, this function is private API and it is always called with field_class=base.FieldABC. Perhaps we could remove that argument.)

[decaz]

The use case is to reuse schema field. For instance it can be useful while declaring flask-apispec schema:

@use_kwargs({'some_field': MySchema.some_field})

Anyway it seems a bit strange to remove class attributes.. What is the reason for?

[danields761]

Can you please elaborate about your use case?

It's not clear to my why someone would want to do that.

Generally speaking, any name declared within class namespace must be accessible after class creation. Obviously, marshmallow.schema.SchemaMeta breaks this rule. You can treat this issue as an attempt to revert normal order.

Also there is official docs which defines this common expectation https://docs.python.org/3/tutorial/classes.html?highlight=class#class-objects

[lafrech]

OK, I think I get it.

You would like the fields not to be removed so that you can access them directly without going through the _declared_fields dict.

I just tested and in fact, not removing the fields (in other words, commenting the if pop section) does not break the tests.

I don't know why the pop was added in the first place. Looks like it could be totally removed.

Overall, we could remove both pop and field_class arguments to _get_fields.

I'm not sure about the side effects and whether or not this should be done in 4.0.

Thanks for pointing this out, already.

[lafrech]

I just opened a PR implementing this: #1631.

[kaijm]

@lafrech I just ran into the same thing :-) Can you please merge the PR?