Closed MarcSkovMadsen closed 2 years ago
Thanks for contacting us. We love hearing from library maintainers, and we appreciate the work you do for the Python community!
The panel
library contains a "py.typed" marker as described in PEP 561, which means that it claims to contain type information. Pyright, the type checker that underlies the pylance language server, treats "py.typed" libraries according to the rules spelled out in this documentation.
If you want a symbol such as Accordion
to be treated as a public symbol available through the top-level module's namespace, you need to do one of the following:
from .layout import Accordion as Accordion
__all__
assignment.
__all__ = ["Accordion", "Card", ...]
It looks like very little of the panel
library contains type annotations. When a library includes a "py.typed" marker, it is claiming that it contains correct and complete type information. The command-line version of pyright includes an option that reports on missing type information in a "py.typed" library. When I run pyright --verifytypes panel
, here is the summary:
Symbols exported by "panel": 3458
With known type: 338
With ambiguous type: 155
With unknown type: 2965
Type completeness score: 9.8%
That means panel
is falling far short of being "typed". You might consider removing the "py.typed" marker until more type declarations are added to the library. Pyright makes different assumptions about libraries that are not marked "py.typed". I think you'll find that it works better if you omit the "py.typed", at least until you make further progress in adding type information.
Longer term, I encourage you to add more type information to the library, follow the guidance set forth in the documentation linked above, and add back the "py.typed" marker.
Hi @erictraut
Thanks. It makes sense and helps.
For context Panel and the HoloViz ecosystem is built on top of Param. Param provides parameters for your Python classes and plays a role similar to traitlets in Jupyter. Its been around since 2003. An example is
import param
class SomeClass(param.Parameterized):
text = param.String(default="hello world", doc="A string attribute")
author = param.String(default="Marc", doc="Name of author", constant=True)
It also works similarly to dataclasses, attrs and pydantic. So all the type information is implicitly there. But static type checkers just don't understand it. But ipython and jupyter understands it really, really well on the other hand and should continue to do so.
The challenge for us to understand which route to take. There seems to be many
__init__
methods to all the Parameterized
classes even though these are autogenerated by Param..pyi
stub files. I can allready do that for all Parameterized
classes. But from tests it seems more is needed.The problem is that finding out which route to take is not something I/ we have the skills to do.
Any advice here would be appriciated.
Regarding __all__
. Should we only include variables, functions and classes? Or also modules?
I've started a PR to add __all__
to the most important __init__.py
files. https://github.com/holoviz/panel/pull/3411/files. Feel free to provide a few helpful suggestions. Thanks.
Should I run pyright --verifytypes panel
with panel
installed as a package or from the root of the cloned panel repository @erictraut?
If you want to participate in static type checking, you'll need to add static type annotations in one form or another — either inline or in the form of type stubs.
I don't have any experience with Param
. It looks like it was designed at a much earlier point in Python's evolution, and it hasn't really kept up with the mainstream Python features and trends. That could make it difficult to adapt to "modern Python". Skimming its docs, it looks like it is minimally maintained at this point.
Then we could create a Param plugin for mypy/ pyright I believe ???
Mypy supports custom plugins, but these plugins are specific to mypy and don't work with any other static type checkers or languages servers within the Python community. We have no plans to add support for plugins in pyright. They are expensive to maintain, difficult for users to discover and install, and represent a security risk. We encourage library authors to stick to the typing standards.
The Python static type standards continue to evolve to support more use cases. For example, we are in the process of ratifying PEP 681 to support attrs, pydantic and other libraries that use dataclass-like semantics. I wonder if that would would be applicable for Param
and panel
?
Regarding all. Should we only include variables, functions and classes? Or also modules?
The purpose of __all__
is to indicate which symbols are imported as part of a wildcard import (e.g. from panel import *
). Many libraries already support wildcard imports, but it's generally considered an antipattern to use wildcard imports when importing from a library because updates to the library can add new symbols that pollute that importer's global namespace in unpredictable ways. PEP 8 recommends against using wildcard imports for this reason.
Since you don't currently use __all__
in panel
, you might want to use the other techniques such as redundant forms of import
statements.
Should I run pyright --verifytypes panel with panel installed as a package or from the root of the cloned panel repository?
You need to run it on the installed package.
Pep 681 is the "unofficial standard I mentioned". I have looked at it a bit. But the learning curve is steep for me. My impression was that Pep 681 was really not very general, but specific for dataclasses, attrs and pydantic. It's thought out in that context to fit those "models". Not really "models" in general?? But I might be wrong. The consequence would be that other models would become "legacy" python if they not already are :-) I've tried to convey that message to the Param maintainers. The same would probably be True for traits and traitlets.
Regarding __all__
vs redundant forms. Something called redundant does not sound attractive :-). Why would I use that over __all__
. That seems just as easy to use? Thx.
The way to really understand PEP 681 and whether it would work with Param or an updated version of Param is probably to try to implement PEP 681 for Param starting here
It looks like it was designed at a much earlier point in Python's evolution, and it hasn't really kept up with the mainstream Python features and trends. That could make it difficult to adapt to "modern Python". Skimming its docs, it looks like it is minimally maintained at this point.
Hello! I guess I'm the most official spokesperson for Param. Thanks for engaging with us!
I'm not sure what about the Param docs is indicating "minimal maintenance" to you (surely that's a question to ask of the git repo or release history instead?), but maybe it's true in the sense that Param is a very mature library that rarely needs changing. So there are occasional fixes and additions, and usually not much more is needed; it's fully suitable for production usage and has been for many years now. We do have a 2.0 release coming up, with some breaking changes that clean up the API, but those would be the first intentionally breaking changes in many years.
As for "modern Python", yes, eventually other mechanisms were added to the language and in other libraries that address some of the needs Param has been addressing since 2003. But there's still nothing else that provides the full set of functionality that Param provides, and unfortunately these partial alternatives used different syntax and sometimes different mechanisms. So now we have a situation where IDEs and other tooling has been built around static type hints that are almost entirely irrelevant in a Param-based approach, because such hints can only provide a tiny bit of what a Parameter supports.
It would be great for the editors to show the user what Param itself already knows, but doing so by adding adding redundant type hints all over user code would destroy a lot of the simplicity that Param provides. But hey, I'm an old Emacs user, so maybe it's just that I wouldn't get any benefit from that, which is why Marc is the one raising this issue rather than me. :-) In any case, if there is a way that we can publish what Param knows in a form that static analyzers can consume, then I'm all for it!
@MarcSkovMadsen, it seems that between this issue and #2574 your questions have been answered. If I'm wrong, let us know.
the learning curve is steep for me
Sorry about that. We'll be providing documentation on docs.python.org in the next few months which hopefully will make it more approachable.
really not very general, but specific for dataclasses, attrs and pydantic
Attrs and pydantic compatibility was important because they are very popular libraries. However, dataclass_transform
is intended to be a general case solution for libraries that implement behaviors like dataclass
.
The way to really understand PEP 681 and whether it would work with Param or an updated version of Param is probably to try to implement PEP 681 for Param
PEP 681 is on track to be approved in Python 3.11. Please try it. If you run into problems or have questions about, please contact me.
Environment data
Pylance language server 2022.1.3 (pyright 98868f26) Windows 10 Enterprise Python 3.9.4 (PSF distribution) VS Code 1.57.1 Panel==0.13.0
Expected behavior (with Jedi)
The above is with
Actual behavior (with Pylance)
As you can see only the sub-modules of the package are available. Not the other objects imported in the
__init__.py
files. For example theAccordion
does not show up when expandingpn.
.pn
__init__.py
file: https://github.com/holoviz/panel/blob/master/panel/__init__.pypn.widgets
__init__.py
file: https://github.com/holoviz/panel/blob/master/panel/widgets/__init__.pyThe above is with
Reproduce
python -m venv .venv
source .venv/Scripts/activate
script.py
filescript.py
file.Additional Context
I'm a contributor to Panel and would like to find the root cause and fix it, if the cause is on the Panel side. I've opened an issue with Panel here https://github.com/holoviz/panel/issues/3400.
Any hints at how to identify the cause would be greatly appreciated. Thanks.
Logs