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

馃摑 Fix ambiguous phrasing regarding `HeroPublicWithTeam` model by berkaykrc 路 Pull Request #1678 路 fastapi/sqlmodel 路 GitHub

Repository navigation

馃摑 Fix ambiguous phrasing regarding HeroPublicWithTeam model - #1678

Merged
svlandeg merged 5 commits into
fastapi:mainfrom
berkaykrc:docs/fix-phrasing
Mar 9, 2026
Merged

svlandeg merged 5 commits into
fastapi:mainfrom
berkaykrc:docs/fix-phrasing

Conversation

Copy link
Copy Markdown
Contributor

Description

I noticed a sentence in the Models with Relationships section that was slightly ambiguous and caused confusion about which model was being described.

In the paragraph discussing recursion, the text says:

"Also, notice that the field team is not declared with this new TeamPublicWithHeroes..."

Because the sentence omits the subject (the HeroPublicWithTeam class), it reads as if the text is describing a field inside TeamPublicWithHeroes, rather than explaining how the team field is typed inside the Hero model.

Changes

I have updated the text to explicitly state that we are looking at the HeroPublicWithTeam model.

Example of clarification:

Before: "Also, notice that the field team is not declared with this new TeamPublicWithHeroes..."

After: "Also, notice that in the HeroPublicWithTeam model, the field team is not declared with this new TeamPublicWithHeroes class..."

This small change reduces cognitive load and prevents readers from looking for a logic error in TeamPublicWithHeroes.

github-actions Bot added the docs Improvements or additions to documentation label Dec 15, 2025

github-actions Bot commented Dec 15, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

YuriiMotov changed the title docs: Fix ambiguous phrasing regarding HeroPublicWithTeam model 馃摑 Fix ambiguous phrasing regarding HeroPublicWithTeam model Dec 24, 2025

YuriiMotov left a comment

Copy link
Copy Markdown
Member

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

@berkaykrc, thanks!

I agree this way it reads a bit easier.

We can also update the paragraph above:

- Now, notice that these new fields `team` and `heroes` are not declared with `Relationship()`, because these are not **table models**, they cannot have **relationship attributes** with the magic access to get that data from the database.
+ Now, notice that these new fields `team` and `heroes` are not declared with `Relationship()`, because `HeroPublicWithTeam` and `TeamPublicWithHeroes` are not **table models**, they cannot have **relationship attributes** with the magic access to get that data from the database.

("these" replaced with explicit "HeroPublicWithTeam and TeamPublicWithHeroes")

svlandeg left a comment

Copy link
Copy Markdown
Member

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

Thanks @berkaykrc! I also applied @YuriiMotov's suggestion, to make both these paragraphs crystal clear 馃帀

svlandeg merged commit d97fef4 into fastapi:main Mar 9, 2026
23 checks passed
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

docs Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants


Back | FazBrowse Home | New Git URL