RemovedInMarshmallow4Warning: Passing field metadata as a keyword arg is deprecated. Use the explicit metadata=... argument instead.

[peterschutt]

Env

marshmallow-sqlalchemy==0.24.1
marshmallow==3.10.0
sqlalchemy==1.3.22

Issue

ModelConverter._add_column_kwargs() passes a "places" keyword argument to fields.Float which isn't part of the fields signature. Until now it would have been silently swallowed into field metadata, however it now generates a warning:

RemovedInMarshmallow4Warning: Passing field metadata as a keyword arg is deprecated. Use the explicit `metadata=...` argument instead.

It checks for the column type to have a scale property:

marshmallow-sqlalchemy/src/marshmallow_sqlalchemy/convert.py

Lines 277 to 278 in ca4c3be

	if hasattr(column.type, "scale"):
	kwargs["places"] = getattr(column.type, "scale", None)

Both Float and Numeric/Decimal column types have the scale property, yet only the Decimal field type accepts the places arg.

The sqlalchemy Numeric type has the asdecimal flag:

    :param asdecimal: default True.  Return whether or not
      values should be sent as Python Decimal objects, or
      as floats.   Different DBAPIs send one or the other based on
      datatypes - the Numeric type will ensure that return values
      are one or the other across DBAPIs consistently.

Changing the above check to this seems to work well:

        if getattr(column.type, "asdecimal", False):
            kwargs["places"] = getattr(column.type, "scale", None)

...which only passes places to the field constructor when sqlalchemy is explicit about using a decimal type.

[sloria]

Good catch! Would you like to send a PR @peterschutt ?

[AbdealiLoKo]

This also happens with the RelatedList

For example: When I use

from marshmallow_sqlalchemy import SQLAlchemySchema

class BookSchema(SQLAlchemySchema):
    class Meta(SQLAlchemySchema.Meta):
        model = BookModel

    authors = auto_field(column='name')

[peterschutt]

@AbdealiJK What are the columns of BookModel in your example? E.g., is one of the fields a float or decimal, or is the warning coming from a different field type?

[AbdealiLoKo]

Here is a fully reproducible example:

from marshmallow_sqlalchemy import SQLAlchemySchema, auto_field
from sqlalchemy import Column, ForeignKey, Integer, String, Table, UniqueConstraint
from sqlalchemy.ext.declarative.api import declarative_base
from sqlalchemy.orm import relationship
BaseModel = declarative_base()

book_author_assoc = Table(
    'book_author_assoc',
    BaseModel.metadata,
    Column('id', Integer, primary_key=True, autoincrement=True),
    Column('book_id', Integer, ForeignKey('book.id', ondelete='CASCADE'), nullable=False),
    Column('author_id', Integer, ForeignKey('author.id'), nullable=False),
    UniqueConstraint('book_id', 'author_id'),
)

class AuthorModel(BaseModel):
    __tablename__ = 'author'
    id = Column(Integer, primary_key=True)
    name = Column(String(255), nullable=False, unique=True)

class BookModel(BaseModel):
    __tablename__ = 'book'
    id = Column(Integer, primary_key=True)
    name = Column(String(255), nullable=False, unique=True)
    authors = relationship(AuthorModel, secondary=book_author_assoc, lazy='selectin')

class BookSchema(SQLAlchemySchema):
    class Meta(SQLAlchemySchema.Meta):
        model = BookModel
    authors = auto_field(column='name') 

And you can run it with: venv/bin/python -W all app.py
I am using: Python 3.7.7, marshmallow 3.10.0, marshmallow-sqlalchemy 0.24.2, SQLAlchemy 1.3.23

[peterschutt]

There's a few more warnings that the test suite throws:

• The same warning as the original example in this issue is thrown by the doc param passed to Student.date_created. The field generator passes that doc attribute through to a description param to the field.
• Same warning, from TestFieldFor.test_field_for from passing session as a kwarg to field_for().
• Again, same warning as above but for passing description param to field constructor in TestPropToFieldClass.test_can_pass_extra_kwargs.
• DeprecationWarning: Passing `info={"marshmallow": ...}` is deprecated. Use `SQLAlchemySchema` and `auto_field` instead.
  The test model Course passes this info parameter to the description column in order to add a validator which generates the warning. Aside from TestModelFieldConversion.test_info_overrides, 4 other tests use the Course model unnecessarily throwing that warning. Perhaps we can remove the info override from the Course model and override that column in the test_info_overrides test to only repro that behavior in the test where it is necessary.
• sqlalchemy.exc.SADeprecationWarning: The Binary class is deprecated and will be removed in a future relase. Please use LargeBinary.
  This warning comes from the test test_convert_types test where it is explicitly confirmed that sa.Binary returns fields.Str. The very next test is for sa.LargeBinary so we can prob just remove the sa.Binary one seeing its depreciated on the sqla side. I don't think this needs to change anything in the library code.

[peterschutt]

Here is a fully reproducible example:

from marshmallow_sqlalchemy import SQLAlchemySchema, auto_field
from sqlalchemy import Column, ForeignKey, Integer, String, Table, UniqueConstraint
from sqlalchemy.ext.declarative.api import declarative_base
from sqlalchemy.orm import relationship
BaseModel = declarative_base()

book_author_assoc = Table(
    'book_author_assoc',
    BaseModel.metadata,
    Column('id', Integer, primary_key=True, autoincrement=True),
    Column('book_id', Integer, ForeignKey('book.id', ondelete='CASCADE'), nullable=False),
    Column('author_id', Integer, ForeignKey('author.id'), nullable=False),
    UniqueConstraint('book_id', 'author_id'),
)

class AuthorModel(BaseModel):
    __tablename__ = 'author'
    id = Column(Integer, primary_key=True)
    name = Column(String(255), nullable=False, unique=True)

class BookModel(BaseModel):
    __tablename__ = 'book'
    id = Column(Integer, primary_key=True)
    name = Column(String(255), nullable=False, unique=True)
    authors = relationship(AuthorModel, secondary=book_author_assoc, lazy='selectin')

class BookSchema(SQLAlchemySchema):
    class Meta(SQLAlchemySchema.Meta):
        model = BookModel
    authors = auto_field(column='name') 

@AbdealiJK the correct parameter to auto_field is column_name:

marshmallow-sqlalchemy/src/marshmallow_sqlalchemy/schema/sqlalchemy_schema.py

Lines 189 to 195 in 6c2edaa

	def auto_field(
	column_name: str = None,
	*,
	model: DeclarativeMeta = None,
	table: sa.Table = None,
	**kwargs,
	):

...that's why column='name' is getting swept into kwargs.

[AbdealiLoKo]

Hi @peterschutt column_name is different from column

Here are the behaviors:

• authors = auto_field(column='name') => it will give me a list of author-names like ['Tolkein']
• authors = auto_field(column_name='authors') => it will give me a list of author-IDS like [1,2,3]
• authors = auto_field() => it will give me a list of author-IDS like [1,2,3] (same as above)

The column attribute is for Related fields, as you see here:

marshmallow-sqlalchemy/src/marshmallow_sqlalchemy/fields.py

Line 44 in 77edf22

def __init__(self, column=None, **kwargs):

The problem seems to be here:

marshmallow-sqlalchemy/src/marshmallow_sqlalchemy/convert.py

Line 150 in 6e43a83

field_kwargs.update(kwargs)

Where the sake **kwargs is being sent to both RelatedList and Related - So, Related works right. But RelatedList shows this warning.

[peterschutt]

Oh yes, I see.. sorry I didn't give your example as much consideration as it deserved.

I've actually been considering how to manage that part of property2field()..

I'm about to send through a PR, let me know if you agree, or not, with the design.

[peterschutt]

There is also this warning: SAWarning: fully NULL primary key identity cannot load any object. This condition may raise an error in a future release. raised from:

marshmallow-sqlalchemy/tests/test_model_schema.py

Lines 716 to 736 in 6c2edaa

	def test_deserialization_of_seminar_with_many_lectures_that_DNE(
	self, models, schemas, session
	):
	seminar_schema = schemas.SeminarSchema()
	seminar_dict = {
	"title": "Novice Training",
	"semester": "First",
	"lectures": [
	{
	"topic": "Intro to Ter'Angreal",
	"seminar_title": "Novice Training",
	"seminar_semester": "First",
	},
	{
	"topic": "History of the Ajahs",
	"seminar_title": "Novice Training",
	"seminar_semester": "First",
	},
	],
	}
	deserialized_seminar_object = seminar_schema.load(seminar_dict, session=session)

Which is a result of the nested lecture objects not having an "id" key.