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

Linting error: account.inbox.filter(...) #1276

Closed
DeflateAwning opened this issue Feb 29, 2024 · 9 comments
Closed

Linting error: account.inbox.filter(...) #1276

DeflateAwning opened this issue Feb 29, 2024 · 9 comments

Comments

@DeflateAwning
Copy link

Describe the bug
The following code gives a linting error:

from exchangelib import Credentials, Account

# from README for completeness
credentials = Credentials("[email protected]", "topsecret")
account = Account("[email protected]", credentials=credentials, autodiscover=True)

# part with linting problem:
email_message = account.inbox.filter(subject__contains='the', sender__contains='example.com').order_by('-datetime_received')[0]

This code runs fine, but the .filter part of account.inbox.filter(...) gets the following linting error in Pyright:

Cannot access member "filter" for type "threaded_cached_property"
  Member "filter" is unknown

Additional context
Python 3.9, exchangelib 5.1.0

@ecederstrand
Copy link
Owner

ecederstrand commented Feb 29, 2024

I think this is just Pyright not being able to understand the code properly. I don't see anything wrong.

@DeflateAwning
Copy link
Author

Just opened an issue at Pyright: microsoft/pyright#7378

@DeflateAwning
Copy link
Author

Welp, pyright is blaming this repo for the error. Looking myself, it looks like it is perhaps an issue here. I think maybe we just need to export the list of attributes in an __all__ = [...] list. I'd have to look at it harder though.

@hmc-cs-mdrissi
Copy link

pydanny/cached-property#172 is probably real issue. threaded_cached_property doesn't play well with static analysis tools in general.

The easiest trick,

if TYPE_CHECKING:
  my_prop = property
else:
  my_prop = threaded_cached_property

@ecederstrand
Copy link
Owner

ecederstrand commented Mar 1, 2024

Most of exchangelib was written long before type annotations became a Python feature. Adding full annotations is a huge effort that I will not undertake myself, at least.

@DeflateAwning
Copy link
Author

I wasn't suggesting that we need full annotations; I do however thing that it makes sense to add hints on parts that are being labelled as errors

@ecederstrand
Copy link
Owner

In this case, it looks like the real issue lies in the cached-property package. I think the issue should be solved there.

There are other places where static type checkers have a hard time reasoning about exchangelib code. I haven't seen any cases where there were actual errors, and refactoring turned out to be a lot of work for very little gain.

I'm closing this issue because exchangelib doesn't use Pyright. I'm not generally opposed to doing so, but please work on adding support in a PR instead.

@DeflateAwning
Copy link
Author

DeflateAwning commented Mar 1, 2024

@ecederstrand, would you be willing to help me write an issue for this in the cached-property package? Or at least point me to the part where that issue needs to be referenced as an example of the issue?

@hmc-cs-mdrissi
Copy link

The issue already exists. pydanny/cached-property#172 is the correct issue. Fixing that will fix the problem here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants