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

Class member is unknown #1117

Closed
pikeas opened this issue Oct 23, 2020 · 5 comments
Closed

Class member is unknown #1117

pikeas opened this issue Oct 23, 2020 · 5 comments
Labels
as designed Not a bug, working as intended

Comments

@pikeas
Copy link

pikeas commented Oct 23, 2020

from pymodm import MongoModel
from pymodm.fields import CharField

class Thing(MongoModel):
    name = CharField()

Pyright sees this as Unknown | Charfield. Pymodm isn't typed, so as an ORM I assume it's confusing Pyright with what it does under the hood. I tried to fix this with:

class Thing(MongoModel):
    name: CharField = CharField()

This remains Unknown | Charfield and I'm not sure why. Shouldn't my explicit annotation definitively tell Pyright what name is?

VS Code extension or command-line
VS Code extension, Pylance 20.10.2

@erictraut
Copy link
Collaborator

When you say that "Pyright sees this as Unknown | Charfield", what expression are you referring to? Is that what you see when you hover over the name symbol? Or is this coming from some other source such as an error message?

I'm not able to repro this with the latest version of Pylance or Pyright. Perhaps I'm using a different version of pymodm than you? I installed the latest version, which appears to be 0.4.3.

@erictraut erictraut added the question Further information is requested label Oct 23, 2020
@pikeas
Copy link
Author

pikeas commented Oct 23, 2020

I've reconfirmed that this is happening as described in a fresh file. I'm running:

  • VS Code 1.50.1
  • Pylance 2020.10.2
  • Pymodm 0.4.3
  • Python 3.8.5 (virtualenv interpreter selected)

Screen Shot 2020-10-23 at 12 11 07

from pymodm import MongoModel
from pymodm.fields import CharField


class Thing(MongoModel):
    name: CharField = CharField()


thing = Thing() # same message for Thing(name='abc')
thing.name

Pylance reports the issue under Problems, and when hovering thing.name.

Here are my Pylance-relevant VS Code settings:

{
    "python.analysis.diagnosticSeverityOverrides": {
      "reportMissingTypeStubs": "none"
    },
    "python.analysis.typeCheckingMode": "strict",
    "python.languageServer": "Pylance",
  }

@erictraut
Copy link
Collaborator

Thanks for the additional information. I'm able to repro the problem now that I have the full code snippet.

First, I noticed that you have "strict" type checking enabled. It's difficult to use strict type checking in Python today because it requires that all imported packages have complete and accurate type information — or that you have found or created type stubs yourself that provide this type information. The 'pymodm' is not a typed library, and I presume that you don't have any type stubs for it. As such, I recommend that you switch from 'strict' to 'basic' typeCheckingMode. You can individually enable some of the other diagnostic checks that are normally off in basic mode, but practically speaking, you won't be able to enable them all.

As for the specific problem you reported, it's due to the fact that the CharField class has a __get__ method, so when you access the field name, this __get__ method is implicitly called. The __get__ method in this case does not have any type annotations, so pyright does its best to infer the return type. If you look at the implementation, it returns either self or a value that is dynamically fetched, and is therefore unknown. That's why the resulting type is Unknown | CharField.

So Pyright is doing the right thing in this case.

@erictraut erictraut added as designed Not a bug, working as intended and removed question Further information is requested labels Oct 23, 2020
@pikeas
Copy link
Author

pikeas commented Oct 23, 2020

Thank you for the quick, thorough, and helpful reply!

You're right about not having custom types for Pymodm. In fact, I encountered this problem while trying to write some initial stubs for it and move my org's ORM models away from # type: ignore. For both Pyright and Mypy (run in CI/CD), I've started from strict and am backing rules off when they're untenable.

This is certainly not correct, but it looks like I can create the following stubs as a rough draft:

# typings/pymodm/base/fields.pyi
from typing import Any, Generic, Optional, TypeVar

T = TypeVar("T")

class MongoBaseField(Generic[T]):
    def __getattr__(self, name: str) -> Any:
        ...  # incomplete

    def __get__(self, inst: Optional[T], owner) -> T:
        ...


# typings/pymodm/fields.pyi
from typing import Any

from .base.fields import MongoBaseField

class CharField(MongoBaseField[CharField]):
    ...

After adding these, Pylance is throwing:

Type of parameter "owner" is unknown

Typeshed says that unannotated arguments are preferred over Any, however Pylance throws reportUnknownParameterType. This is technically correct but is it working as intended? Should Pylance skip that check for type-only .pyi files?

Pylance is also throwing reportUnknownMemberType for typings/pymodm/__init__.pyi with __gettattr__. Similar to the above, this is technically correct but failing when following typeshed's guidelines for partially typed modules.

Please let me know if you prefer a new issue for this.

@erictraut
Copy link
Collaborator

owner is a parameter. Because you've specified that you want "strict" type checking, all parameters must have annotations.

PEP 484 indicates that parameters that have no annotation should be treated as though they are "Any". Pyright goes a step further and differentiates between an explicit "Any" and an implicit "Any". It refers to the latter as "Unknown". When in strict mode, Pyright emits errors if there are any Unknowns.

As I mentioned above, "strict" mode is difficult to achieve, especially in an existing code base. I recommend that you start with "basic" mode and then enable more diagnostic rules as you work your way up to "strict". Here is some documentation that you might find useful.

Baschdl added a commit to Baschdl/sbi that referenced this issue Mar 19, 2024
Baschdl added a commit to Baschdl/sbi that referenced this issue Mar 19, 2024
Baschdl added a commit to sbi-dev/sbi that referenced this issue Mar 20, 2024
…me bugs (#1045)

* Exclude tests from pyright when executing from bash

* Disable general type issues as we could only use typed libraries otherwise. See microsoft/pyright#1117 (comment) for details

* Pin to newest version but restrict to basic type checking to have a manageable number of errors

* Add failing test addressing reportAttributeAccessIssue regarding scipy's rv_frozen/multi_rv_frozen @janfb

* Correct return type for standardizing_transform

* Let pyright know that the kwargs could have the right type for the downstream arguments (reportArgumentType)

* Make sure that torch.cat only gets Tensors as input but ignore unknown return type of Parallel generator (reportAssignmentType)

* Specify numpy array sequence

* Type return of subplot, ignore that we could get multiple Axis as we know that there will be only one

* Specify additional type NFlowsFlow for continuous_net

* Ignore OptionalOperand and OptionalMemberAccess as we know that matplotlib has those propertoes

* Ignore that matplotlib's types are complicated with subplots

* Ignore wrongly inferred type of matplotlib

* Change Figure to FigureBase, ignore subplots

* allow usage of alias types (introduced for documentation)

* Ignore once again that Parallel doesn't return the right type

* Correct type hint and fix documentation

* Ignore as we know that there'll be only one or two return values

* Ignore as import exists

* Fix type for list of Transforms

* Ignore errors despite mismatch in Types but as the code works I would suspect that they are compatible because they're all based on nn.Modules

* Revert "Specify additional type NFlowsFlow for continuous_net"

This reverts commit 02b0c7e.

* Bump pyright version

* ConditionedMDN should take NFlowsFlow

* Fix import of sbi_types

* Assign type as we know that this is a Flow

* mnle sample fix

* Fix kwargs from main and add comment to CONTRIBUTING

* Deprecate the possibility to pass a scipy object, we now require a pytorch Distribution

---------

Co-authored-by: Guy Moss <guy.moss13@gmail.com>
Co-authored-by: Guy Moss <91739128+gmoss13@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
as designed Not a bug, working as intended
Projects
None yet
Development

No branches or pull requests

2 participants