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.

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

Support one-to-many relationships with ModelView.column_type_formatters #347

Open
1 task done
lenzo202 opened this issue Oct 2, 2022 · 4 comments
Open
1 task done

Comments

@lenzo202
Copy link

lenzo202 commented Oct 2, 2022

Checklist

  • There are no similar issues or pull requests for this yet.

Is your feature related to a problem? Please describe.

I often want to display the list of related models for one-to-many relationship so that they can easily be navigated to via the links. The default formatting of printing all model attributes is often unreadable, and is sometimes unusable for larger models.

This is easily solved for one-to-one relationships or the "one-side" of a one-to-many relationship by updating the views column_type_formatters with a format function which returns a readable representation. This does not work for the "many-side" of a one-many relationship because ModelView._default_formatter only checks the "outer type", which returns list. I would expect the formatter to also check the "inner type", which in the below example would be Hero for Team.heros.

class Team(SQLModel, table=True):
    id: Optional[int] = Field(default=None, primary_key=True)
    name: str = Field(index=True)
    headquarters: str

    heroes: List["Hero"] = Relationship(back_populates="team")


class Hero(SQLModel, table=True):
    id: Optional[int] = Field(default=None, primary_key=True)
    uuid: UUID = Field(default_factory=uuid4)
    name: str = Field(index=True, max_length=5)
    secret_name: str
    age: Optional[int] = None

    team_id: Optional[int] = Field(default=None, foreign_key="team.id")
    team: Optional[Team] = Relationship(back_populates="heroes")


def HeroAdmin(ModelView, model=Hero):
    column_list = [Hero.name, Hero.team]


def team_format(team:Team):
    return team.name

# This works
HeroAdmin.column_type_formatters.update({Team: team_format})


class TeamAdmin(ModelView, model=Team):
    column_list = [Team.name, Team.heroes]]


def hero_format(hero:Hero):
    return hero.name

# this does not work
TeamAdmin.column_type_formatters.update({Hero: hero_format})

Describe the solution you would like.

I would like to see an implementation like the the below for ModelView._default_formatter. I believe this to be a robust solution as the existing behavior is maintained, even if a formatter for type list is registered with the model. If someone specifies a formatter for T on a model which specifies a values: List[T] = Relationship(back_populates="something") field, I think it is safe to assume they expect that formatter to work for that field. It also works for an empty list.

class ModelView(BaseView, metaclass=ModelViewMeta):
...
    def _default_formatter(self, value: Any) -> Any:
        if type(value) in self.column_type_formatters:
            formatter = self.column_type_formatters[type(value)]
            return formatter(value)
        if isinstance(value, list) and value:
            inner_type = type(value[0])
            if inner_type in self.column_type_formatters:
                formatter = self.column_type_formatters[inner_type]
                return [formatter(val) for val in value]
        return value
....

Describe alternatives you considered

A potential workaround which requires no changes is to specify a formatter for type list, and use a bunch of checks within the function like the below. This doesn't feel like a clean solution to me though.

def list_format(list_item:list):
    if not list_item:
        return
    inner_type = type(list_item[0])
    if inner_type is Hero:
        return [hero.name for hero in list_item]
    if inner_type is OtherType:
        return [thing.id for thing in list_item]
# etc....
    return list_item

Additional context

@lenzo202
Copy link
Author

lenzo202 commented Oct 2, 2022

went ahead and opened PR #348

@aminalaee
Copy link
Owner

Hey,
Thanks for the great explanation. I agree that this is a common use case.

But just an idea, does it make sense to use column_formatter instead of column_type_formatters?

@lenzo202
Copy link
Author

lenzo202 commented Oct 2, 2022

Hey, Thanks for the great explanation. I agree that this is a common use case.

But just an idea, does it make sense to use column_formatter instead of column_type_formatters?

For this example I think column_formatter would have been simpler. I think it might still be nice to have this type formatter as well for a few reasons.

  • if I want to use this formatter for the type globally, I could update BASE_FORMATTERS or loop over the views at the app level. Not sure if theres an equivalent to this for column_formatters as theres no guarantee the attribute names will be the same across models.
  • type formatters apply to list and detail views. The docs suggest there are separate column_formatters and column_formatters_detial. Haven't looked into the code for these though.

@aminalaee
Copy link
Owner

type formatters apply to list and detail views. The docs suggest there are separate column_formatters and column_formatters_detial. Haven't looked into the code for these though.

Yes that's right.
Yeah probably worth thinking about. Thanks for the suggesiton.

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

No branches or pull requests

2 participants