plotly / dash

Data Apps & Dashboards for Python. No JavaScript Required.
https://plotly.com/dash
MIT License
21.11k stars 2.04k forks source link

Support Dynamic Attributes and IDE inspection (type hints) #2494

Open eric-spitler opened 1 year ago

eric-spitler commented 1 year ago

Is your feature request related to a problem? Please describe. Not a functional problem, but rather PyCharm inspection reports potential issues due to the use of dynamic attribute assignment of many Python classes. The attributes can exist, but since they are not listed as type hints on the class or set directly during __init__, the IDE doesn't not know which attributes are valid for a given class.

For example, the dash.dcc.Dropdown class (along with most others) has a id attribute that can be set during instantiation. This is listed as an argument in __init__ method signature; it is not assigned to the instance via self.id = ..., but instead by setattr(self, k, v) in base_component.py. As a result, the IDE knows that it is an argument, but not that the attribute exists on the instance or what its type is. When accessing my_dropdown.id in code, PyCharm flags this as an UnresolvedReferences warning.

Describe the solution you'd like There are number of ways to resolve this: 1) Type hints on classes to indicate which attributes will exist after dynamic assignment and their potential type(s) 2) Add @DynamicAttrs to the docstring to ignore these warnings on a class 3) Explicitly set during __init__.py using self.id = ... and similar

The use of dynamic attribute setting is clearly an intentional design choice, so option 3 is counter-productive.

Option 2 is a quick addition to the docstring that will cause PyCharm to ignore these references, but in so doing it means using my_dropdown.some_bogus_attr would not be flagged, and potentially create an error.

Option 1 is more deliberate about indicating the attributes, and therefore requires more code on the class definition, but would increase support for IDEs with type checking and inspections. It would require one line per attribute:

from typing import Dict, List, Literal, Optional, Union

class Dropdown(Component):
    options: Optional[List[Union[str, float, int, bool, dict]]]
    value: Optional[Union[str, float, in, bool, List[str, float, int, bool]]]
    multi: bool
    clearable: bool
    searchable: bool
    search_value: Optional[str]
    placeholder: Optional[str]
    disabled: bool
    optionHeight: int
    maxHeight: int
    style: Optional[Dict[str, Union[int, float, str, bool]]]
    className: Optional[str]
    id: Optional[str]
    loading_state: Optional[Dict[Literal['component_name', 'is_loading', 'prop_name'], Union[str, bool]]]
    persistence: Optional[Union[bool, str, int]]
    persisted_props: List[str]
    persistence_type: str

Also, by adding the type hints, the __init__ method can make use of typing.get_type_hints to determine the possible attributes, rather than setting self.available_properties. It can even go a step further and check the type of the value being assigned is valid, raising TypeError during instantiation if the user is supplying an incorrect type.

Describe alternatives you've considered A developer using dash and plotly can mark # noinspection or # noqa on the lines where an attribute is accessed, but this requires flagging each line/function/class in every project. Furthermore, disabling code inspections can create problems later after a dependency update if an expected attribute is no longer present. The application must be run to hit an AttributeError rather than detecting it at development time.

Additional context

eric-spitler commented 1 year ago

Related #1748

eric-spitler commented 1 year ago

Related #2276