sdispater / pendulum

Python datetimes made easy
https://pendulum.eustace.io
MIT License
6.25k stars 387 forks source link

pendulum.parse is not marked as exported (pyright type checker in VSCode) #595

Open rnovacek opened 2 years ago

rnovacek commented 2 years ago

Feature Request

pyright type checker in VSCode reports a reportPrivateImportUsage error on following:

import pendulum
pendulum.parse('')

According to pyright docs "Imported symbols are considered private by default." Which is the case of pendulum.parse.

It seems that simply changing the line to named export resolves the problem.

-from .parser import parse
+from .parser import parse as parse

This is the case for all imported symbols, but I came across it on pendulum.parse. I'm aware it's probably just pyright-specific issue, so I'm reporting it as Feature Request. I'm also willing to create a PR if you consider this a valid request.

rnovacek commented 2 years ago

I should have said that you need to set python.analysis.typeCheckingMode to basic in VSCode settings in order to see the error.

gwerbin commented 1 year ago

Mypy reports the same error. Another option in general is to define __all__ with all "exported" names.

eblume commented 11 months ago

This issue also occurs in pyright with no VSCode specific configuration (IE pipx install pyright && pyright example.py). Pyright issues tracks the general case here: https://github.com/microsoft/pyright/issues/2277#issuecomment-937468789 - but as you will see, they consider this closed.

I'm not sure I follow the entire logic of the 'typed-libraries.md' document linked in that comment, and I note that parser is already present in the __init__.py::__all__ file here: https://github.com/sdispater/pendulum/blob/75a87a4e3cab45c5f9c38ca6060a54337dcadd09/src/pendulum/__init__.py#L385C8-L385C8, so I don't know if @gwerbin 's option works here. (edit note: as @tekumara points out below, this is incorrect - the change I've linked here is not yet part of a released stable version. IE, this will be fixed in 3.0. Thanks!)

Does anyone understand the specific logic here? Is this an upstream bug for pyright seperate from issue 2277? It seems to my like pyright isn't respecting pendulum's init.py::__all__.

It's possible another option here is to reword all of the pendulum imports in that file to use relative paths, ie. from .parser import parse instead of from pendulum.parser import parse, which I personally also feel is safer as I understand relative imports in libraries to avoid pitfalls with, for instance, working on editable libraries while non-editable versions exist on PYTHONPATH. But I'll be honest here: I find all of this confusing, and I would love to have someone smarter than me explaining it.

For now, I'm fixing this with a simple from pendulum.parser import parse on my end.

tekumara commented 11 months ago

@eblume pyright respects __all__ and installing the commit you linked resolves the issue, eg:

pip install git+https://github.com/sdispater/pendulum.git@75a87a4e3cab45c5f9c38ca6060a54337dcadd09

I think we just need a new release that contains this commit (as its not present in version 2.1.2)