FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Emit warning on direct instantiation of unmapped MappedAsDataclass by VimalN2005 · Pull Request #13587 · sqlalchemy/sqlalchemy · GitHub

Repository navigation

Emit warning on direct instantiation of unmapped MappedAsDataclass - #13587

Open
VimalN2005 wants to merge 1 commit into
sqlalchemy:mainfrom
VimalN2005:fix-10064-mapped-as-dataclass-warn
Open

VimalN2005 wants to merge 1 commit into
sqlalchemy:mainfrom
VimalN2005:fix-10064-mapped-as-dataclass-warn

Conversation

Copy link
Copy Markdown

Fixes

Fixes: #10064

Description

Direct instantiation of unmapped classes (mixins or abstract base classes) using MappedAsDataclass is unsupported as ORM features, column expressions, and relationship attributes cannot be resolved without a concrete mapping.

Per issue discussion and maintainer guidance:

  • Installed an initialization wrapper in _UnmappedDataclassConfig for classes subclassing MappedAsDataclass that emits an SAWarning when the unmapped class is directly instantiated (type(self_) is cls_).
  • Concrete mapped subclasses remain unaffected and instantiate their own dataclass constructors normally without warning.

Checklist

  • Added unit tests in test/orm/declarative/test_dc_transforms.py (UnmappedDataclassInstantiationTest)
  • All 370 tests in test_dc_transforms.py and test_dataclasses.py pass cleanly
  • Added changelog fragment doc/build/changelog/unreleased_21/10064.rst

Copy link
Copy Markdown
Collaborator

Thanks! Issue #10064 is now marked code review in progress and no longer open for pull requests, so this pull request holds the review for it and another one won't land on top of your work. If this pull request is abandoned, a maintainer can put open for pull requests back on #10064 to reopen it to others.

zzzeek requested a review from sqla-tester September 24, 2026 13:04

sqla-tester left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision b9e5934 of this pull request into gerrit so we can run tests and reviews and stuff

Copy link
Copy Markdown
Collaborator

New Gerrit review created for change b9e5934: https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/6991

VimalN2005 force-pushed the fix-10064-mapped-as-dataclass-warn branch from b9e5934 to c72f83b Compare September 24, 2026 14:16

Copy link
Copy Markdown
Author

Hi @zzzeek,

I have addressed the PEP8 (line length / formatting) and PEP484 (typing in decl_base.py) issues flagged by the Gerrit verification pipeline:

  1. PEP8 Compliance: Fixed line lengths in decl_base.py and test_dc_transforms.py, formatted cleanly with black and verified with flake8.
  2. PEP484 Typing: Corrected util.warn() invocation without redundant category argument and used setattr for dynamic __init__ wrapping, resolving all mypy errors.
  3. Unit Tests: All dataclass transform unit tests pass cleanly.

The branch has been updated with commit c72f83b. Ready for Gerrit re-verification. Thank you!

sqla-tester left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Hi, this is sqla-tester and I see you've pinged me for review. However, user VimalN2005 is not authorized to initiate CI jobs. Please wait for a project member to do this!

zzzeek requested a review from sqla-tester September 24, 2026 17:06

sqla-tester left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision c72f83b of this pull request into gerrit so we can run tests and reviews and stuff

Copy link
Copy Markdown
Collaborator

Patchset c72f83b added to existing Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/6991

sqla-tester left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Federico Caselli (CaselIT) wrote:

Loiks fine. The sync teat script may require to be run

View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/6991

Direct instantiation of unmapped classes (mixins or abstract base
classes) using MappedAsDataclass is unsupported as ORM features and
attributes cannot be resolved without concrete mapping.

An SAWarning is now emitted when instantiating such classes directly.
Concrete mapped subclasses remain unaffected and instantiate normally.

Fixes: sqlalchemy#10064
Change-Id: I5875469671fce1f63e7e93776683e4ba9374c59b
VimalN2005 force-pushed the fix-10064-mapped-as-dataclass-warn branch from c72f83b to 6031fac Compare September 24, 2026 18:51

Copy link
Copy Markdown
Author

Hi @CaselIT @zzzeek,

I have addressed both review items:

  1. Preload Decorator: Added @util.preload_module("sqlalchemy.orm.decl_api") on _install_unmapped_dataclass_init_warning.
  2. Synchronized Test Suite: Ran tools/sync_test_files.py to synchronize test_dc_transforms_future_anno_sync.py.
  3. Verification: Both normal and future-annotations test suites pass cleanly (6/6 tests), formatted with black and verified with flake8 and mypy.

Updated in commit 6031fac. Ready for review/CI run. Thank you!

sqla-tester left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Hi, this is sqla-tester and I see you've pinged me for review. However, user VimalN2005 is not authorized to initiate CI jobs. Please wait for a project member to do this!

zzzeek requested a review from sqla-tester September 24, 2026 19:22

sqla-tester left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 6031fac of this pull request into gerrit so we can run tests and reviews and stuff

Copy link
Copy Markdown
Collaborator

Patchset 6031fac added to existing Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/6991

sqla-tester left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Federico Caselli (CaselIT) wrote:

code review left on gerrit

View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/6991

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

MappedAsDataclass, prohibit constructor use and/or creation for non-mapped classes, abstract, mixins

2 participants


Back | FazBrowse Home | New Git URL