villebro commented on pull request #11883: URL: https://github.com/apache/incubator-superset/pull/11883#issuecomment-739910730
> > Would now be a good time to move these to a separate file, like `superset/types.py`? I would also consolidate the aliases from `superset/typing.py` into the new file, as that name clashes with the builtin `typing`. > > Do we consider enums part of typing? I thought they are constants as well. Hmm, I've always thought of enums as being types, and strictly speaking they are in Python, anyway: ```python >>> from enum import Enum >>> class MyEnum(Enum): ... ABC = 1 ... >>> abc = MyEnum.ABC >>> type(abc) <enum 'MyEnum'> ``` But if the general convention is to call them constants I'm fine with that, too. > Was debating internally whether to move them into `superset.constants`, but kind of also feel for less general enums, they should probably be put locally into each submodule. It's a hard choice, so I went with the easiest and smallest change... > > Either way, we should definitely find ways to break up `core.py` files that have thousands of lines of code. I agree, the specific types should probably be put in the correct submodule, as right now `utils/core.py` is really bloated with these. But that can be saved for a later PR. > Could you clarify how does `superset.typing` clash with the builtin `typing`? I've seen quite a few reports where `import x from typing` is picking up `superset.typing`. Here's a recent example: https://github.com/apache/incubator-superset/issues/11925 . This once happened to me, too, but not sure what was causing it, probably something with an incorrect `PYTHONPATH`. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
