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.

Sign up for GitHub

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

Jump to bottom

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

Open
2 tasks done
rnovacek opened this issue Jan 3, 2022 · 4 comments · May be fixed by #693
Open
2 tasks done

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

rnovacek opened this issue Jan 3, 2022 · 4 comments · May be fixed by #693
Labels
Code Quality Typing

Comments

@rnovacek
Copy link

rnovacek commented Jan 3, 2022

  • I have searched the issues of this repo and believe that this is not a duplicate.
  • I have searched the documentation and believe that my question is not covered.

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
Copy link
Author

rnovacek commented Jan 3, 2022

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

@tekumara tekumara mentioned this issue Oct 18, 2022
Closed
1 task
cosmix added a commit to cosmix/pendulum that referenced this issue Feb 28, 2023
@cosmix
fixup: Fix sdispater#595
ad5dbd3
@cosmix cosmix linked a pull request Feb 28, 2023 that will close this issue
Open
@gwerbin
Copy link

gwerbin commented Oct 25, 2023

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

@eblume
Copy link

eblume commented Nov 25, 2023 •
edited

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: microsoft/pyright#2277 (comment) - 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
Copy link

tekumara commented Dec 2, 2023

@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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Code Quality Typing
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fixup: Fix #595 cosmix/pendulum
5 participants
@tekumara @rnovacek @eblume @gwerbin @Secrus