`Field(nullable=True)` is ignored due to recent changes released in version 0.0.7

[br-follow]

First Check

• I added a very descriptive title to this issue.
• I used the GitHub search to find a similar issue and didn't find it.
• I searched the SQLModel documentation, with the integrated search.
• I already searched in Google "How to X in SQLModel" and didn't find any information.
• I already read and followed all the tutorial in the docs and didn't find an answer.
• I already checked if it is not related to SQLModel but to Pydantic.
• I already checked if it is not related to SQLModel but to SQLAlchemy.

Commit to Help

• I commit to help with one of those options 👆

Example Code

from typing import Optional

from sqlmodel import Field, Session, SQLModel, create_engine

class Hero(SQLModel, table=True):
  id: Optional[int] = Field(default=None, primary_key=True)
  # This should not be nullable
  name: Optional[str] = Field(default=None, nullable=False)

hero = Hero()
engine = create_engine("sqlite:///database.db")

SQLModel.metadata.create_all(engine)
with Session(engine) as session:
  session.add(hero)
  session.commit()
  session.refresh(hero)
  print(hero)

Description

• Create a hero model with a non-nullable column by assigning it to Field(nullable=True)
• Create a hero instance without setting the column
• Save the instance to the DB. Should get an error but the hero instance saves successfully.

I believe this issue was introduced here: 9830ee0#r82434170

Operating System

macOS

Operating System Details

No response

SQLModel Version

0.0.7

Python Version

Python 3.10.5

Additional Context

No response

[JonasKs]

EDIT: See comment below. I thought it was iffy due to tests not running properly.

Yeah, there's something iffy here, but I can't reproduce your example.

In the test, it is asserted that name VARCHAR NOT NULL.

The test above uses this code, and it still pass when changed into this:

from typing import Optional

from sqlmodel import Field, SQLModel, create_engine


class Hero(SQLModel, table=True):
    id: Optional[int] = Field(default=None, primary_key=True)
-   name: str
+   name: Optional[str] = Field(default=None, nullable=False)
    secret_name: str
    age: Optional[int] = None


sqlite_file_name = "database.db"
sqlite_url = f"sqlite:///{sqlite_file_name}"

engine = create_engine(sqlite_url, echo=True)

SQLModel.metadata.create_all(engine)

Which is good.

However, it also passes when changing name in to nullable=True. So nullable properties have some weird behavior.

from typing import Optional

from sqlmodel import Field, SQLModel, create_engine


class Hero(SQLModel, table=True):
    id: Optional[int] = Field(default=None, primary_key=True)
-   name: str
+   name: Optional[str] = Field(default=None, nullable=True)
    secret_name: str
    age: Optional[int] = None


sqlite_file_name = "database.db"
sqlite_url = f"sqlite:///{sqlite_file_name}"

engine = create_engine(sqlite_url, echo=True)

SQLModel.metadata.create_all(engine)

[JonasKs]

I've submitted a new test that tests nullable fields, and it seems correct to me.
See PR #423.

[br-follow]

I've submitted a new test that tests nullable fields, and it seems correct to me. See PR #423.

I believe I have confirmed and addressed the issue in #424.

[JonasKs]

Aha, I understand now. Seems like a weird pattern to set a default value, but not allow it.. To me it seems like you just shouldn't type it as optional.

As a side note, please either branch out from the PR / ask for branch permissions to add more commits / add the original author as a co-author when you copy/paste code from another PR 😊 You've copied everything I wrote, but I am not listed as a contributor. 😉

[br-follow]

You might type is as optional so that you can create the object without setting all the fields.

Happy to do either. If you give me branch permissions, I can make my change to your existing branch.

Aha, I understand now. Seems like a weird pattern to set a default value, but not allow it.. To me it seems like you just shouldn't type it as optional.

As a side note, please either branch out from the PR / ask for branch permissions to add more commits / add the original author as a co-author when you copy/paste code from another PR 😊

[br-follow]

You might type is as optional so that you can create the object without setting all the fields.

Happy to do either. If you give me branch permissions, I can make my change to your existing branch.

Aha, I understand now. Seems like a weird pattern to set a default value, but not allow it.. To me it seems like you just shouldn't type it as optional.
As a side note, please either branch out from the PR / ask for branch permissions to add more commits / add the original author as a co-author when you copy/paste code from another PR 😊

Added you as a co-author: 00cebd2

[JonasKs]

Gave you access to mine, but up to you. I assume you have to rebase those commits into one commit to get it merged anyway 😄

[JonasKs]

Since you added me as a co-author on the wrong commit, I honestly think you should just finish your PR, copy over the changes you'd like into my PR, and then close yours. Then git history will be correct, and git commit history could be two commits with the correct authors.

[br-follow]

I assumed @tiangolo would squash the commits before merging the PR. But I will commit to your PR.