incorrect typing in horizontal_alignment and vertical_alignment

[Vlod-github]

flet/sdk/python/packages/flet-core/src/flet_core/view.py

Line 145 in 969cb22

def vertical_alignment(self, value: MainAxisAlignment):

Hi, the vertical_alignment function accepts value: MainAxisAlignment. But in fact it accepts it accepts value: Union[MainAxisAlignment, MainAxisAlignmentString]. Similarly with horizontal_alignment

Insert the following code

from typing import List, Optional, Union

def horizontal_alignment(self, value: Union[CrossAxisAlignment, CrossAxisAlignmentString]):

def vertical_alignment(self, value: Union[CrossAxisAlignment, CrossAxisAlignmentString]):

[Vlod-github]

1. The problem is that the fields that are identical in meaning give different values when compared. It is expected that either such fields cannot be set or they are the same when compared.

from flet_core import Row, MainAxisAlignment

row_a = Row(alignment=MainAxisAlignment.START)
row_b = Row(alignment="start")
assert row_a.alignment == row_b.alignment #  Error

Decision. We will replace Enum with StrEnum.

from enum import Enum, StrEnum

class TestEnum(StrEnum):
    START = 'start'

print('start' == TestEnum.START)  # Ok

[Vlod-github]

2. Now vertical_alignment accepts Enum, but there is logic for working with strings. It was also suggested to leave the possibility of passing a string.

    @property
    def vertical_alignment(self) -> CrossAxisAlignment:
        return self.__vertical_alignment

    @vertical_alignment.setter
    def vertical_alignment(self, value: CrossAxisAlignment):
        self.__vertical_alignment = value
        if isinstance(value, CrossAxisAlignment):
            self._set_attr("verticalAlignment", value.value)
        else:
            self.__set_vertical_alignment(value)

I suggest removing the detailed description of Literal CrossAxisAlignmentString and others, because:

1. They duplicate information from Enum.
2. We will not be able to properly connect Literal and Enum without triple duplication of information.
   Therefore, we must remove the CrossAxisAlignmentString, if developers want to see which strings the function can accept, then they can look in the Enum or documentation.

To validate the input parameters, I suggest using the helper function:

def enum_to_str(enum: StrEnum, value: Union[StrEnum, str]) -> Optional[str]:
    if isinstance(value, StrEnum):
        return value.value
    if value in enum.__dict__['_value2member_map_']:
        return value
    return None

This function will either return the correct Enum value, or None. This can be useful because Control stores strings.

Also earlier we allowed the use of incorrect arguments for objects, for example:

Row(alignment="center")
Row(alignment="CENTer")
Row(alignment="CenteR")

The code continued to work as if this was normal behavior. Now, in case of an incorrect value, the value None will be set. In the future, we can add logging or runtime error if necessary.

[Vlod-github]

Write what you think about 1. and 2. And depending on the answer, we can update the code.
@ndonkoHenri, @FeodorFitsner

[gbrandt]

My thought is we should enforce the use of enums and disallow typing strings directly. That's what enums are for, to remove the complexity of value from the user. However I am not sure of what is 'Pythonic' in a case like this. Is it normal for the developer to be allowed to use the enum or the string interchangeably on assignment? I come from a C++ world and this would not be allowed in modern C++

[FeodorFitsner]

I agree with @gbrandt we should get rid of strings in typings. Flet is still pre 1.0, so, theoretically, it's "allowed" to introduce breaking changes to its API ☺️

I like #2 solution suggested by @Vlod-github here - i.e. leave enum typings only in property declarations and remove {something}String literal typings everywhere; "inside" existing properties assigning both enums and strings will be supported to avoid breaking existing code (linter will be showing errors though, but it's OK). All new properties will be done to accept enum values only.

@Vlod-github would you willing to help with a PR for that?

[Vlod-github]

Yes, I will do it on the weekend

[Vlod-github]

I ran into problems due to the fact that Enums contain values of different types.

1. For the Enum class, enum.value returns Any, so I had to complicate the function code:

def enum_to_str(enum: Enum, value: Union[Enum, str]) -> Optional[str]:
    if isinstance(value, Enum):
        if isinstance(value.value, str):
            return value.value
    elif value in enum.__dict__['_value2member_map_']:
        return value
    return None

2. Consider converting from string back to Enum:

def value_to_enum(enum: Type[T], value) -> Optional[T]:
    if isinstance(value, Enum):
        return value  # type: ignore
    return enum.__dict__['_value2member_map_'].get(value)

def vertical_alignment(self) -> Optional[CrossAxisAlignment]:
    return value_to_enum(CrossAxisAlignment, self.__vertical_alignment)

def vertical_alignment(self, value: CrossAxisAlignment):
	_value: Optional[str]
	_value = enum_to_str(CrossAxisAlignment, value)
	self.__vertical_alignment = _value
	self._set_attr("verticalAlignment", _value)

The vertical_alignment function can now return both Enum.NONE and None explicitly. We have a duplication of None, because the external code does not know if the abstract Enum has .NONE.