Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Double pluralization of names #262

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Double pluralization of names #262

wants to merge 11 commits into from

Conversation

dkratzert
Copy link

@dkratzert dkratzert commented Mar 22, 2023 •
edited
Loading

Fixes #260 Stop double pluralization of names

dkratzert and others added 4 commits March 22, 2023 10:49
@dkratzert
prevent relationship names to be double pluralized
9b2d306
@dkratzert
Add test for double pluralized relationship
6ca088f
@dkratzert
Add test for singular table name
2448a78 
* Makes sure that a singular table name with a one-to-many relationship does not end up with a relationship name of 'False' because of self.inflect_engine.singular_noun(preferred_name) returns False with a singular preferred_name in generate_relationship_name().
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
1f6f2cb 
for more information, see https://pre-commit.ci
@coveralls
Copy link

coveralls commented Mar 22, 2023 •
edited
Loading

Coverage Status

coverage: 97.074%. remained the same
when pulling 35f11ee on dkratzert:double-pluralization-of-names
into b60293f on agronholm:master.

@dkratzert dkratzert marked this pull request as ready for review March 22, 2023 12:27
@agronholm
Copy link
Owner

Regarding the first test, simple_item = relationship('SimpleItem', back_populates='container') is not correct because the related foreign key column does not have a unique constraint on it.

@NixBiks
Copy link

NixBiks commented Nov 16, 2023 •
edited
Loading

@dkratzert thanks for this PR. Are you still planning to get this merged?

@dkratzert
Copy link
Author

Sorry for the delay! I hope to have time for it soon.

@sheinbergon
Copy link
Collaborator

sheinbergon commented Mar 10, 2025 •
edited
Loading

@dkratzert we are now ready to deal with this issue. We need one or two tests to cover this change, (including the FK case @agronholm commented about). Can you re-add them?

@agronholm
Copy link
Owner

@dkratzert we are now ready to deal with this issue. We need one or two tests to cover this change, (including the FK case @agronholm commented about). Can you re-add them?

We could just resurrect and correct the tests from the commit history.

@dkratzert
Copy link
Author

Please use the tests from the history. I still don't find the time...

@sheinbergon sheinbergon self-assigned this Mar 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@sheinbergon sheinbergon

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Stop double pluralization of names
5 participants
@dkratzert @coveralls @agronholm @NixBiks @sheinbergon