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

Mypy no_implicit_reexport x testfixtures #178

Closed
g-as opened this issue Nov 30, 2022 · 11 comments
Closed

Mypy no_implicit_reexport x testfixtures #178

g-as opened this issue Nov 30, 2022 · 11 comments

Comments

@g-as
Copy link

g-as commented Nov 30, 2022

Hi,

when enabling no_implicit_reexport, mypy displays:

***.py:98:5: error: "Module testfixtures" does not explicitly export attribute "compare"  [attr-defined]

It happens whether from testfixtures import compare or import testfixures; testfixtures.compare(..., ...).

According to the docs, you either have to have in __init__.py from testfixtures.comparison import compare as compare or __all__ = (..., "compare", ...).

Would you be willing to modify the __init__.py in order to address this?

If so, I can draft a PR depending on which way you prefer (either with __all__ or with from foo import bar as bar).

Some context/examples on this particular issue:
python/mypy#10198
konradhalas/dacite#133
konradhalas/dacite@a7834ba
pallets/quart@c219d96

In any case, a way to disable this mypy flag only for testfixtures is with:

[[tool.mypy.overrides]]
module = "testfixtures"
implicit_reexport = true
@cjw296
Copy link
Member

cjw296 commented Dec 2, 2022

@g-as - Would this problem be resolved if the py.typed file if removed from the testfixtures distribution?

I'm afraid I don't have time or energy to appease mypy on the testfixtures project. The type annotations that are there are intended to be helpful documentation and off aid to IDEs...

FWIW, that no_implicit_reexport option looks pretty bogus, why are you enabling it?

@g-as
Copy link
Author

g-as commented Dec 2, 2022

Hi,

from my understanding, py.typed just declares to the lib users: "you can look at my annotations", so it's a separate issue. Not even sure it's linked to annotations per se.

My use no_implicit_reexport is twofold:

  • making sure when I want to export a sort of public API through imports in an __init__.py, I do it properly. When enabling the flag, it allowed me to fix some missing import. That's the "use case" of this particular issue.
  • making sure I'm not importing from a module something that was imported there to be used there, and instead use the direct import path:
a.py

from some_lib import some_stuff

use_some_suff()

Works, but not necessarily intended:

b.py

from a import some_stuff

If I indeed do want to "reexport" explicitely some_stuff for use in b.py =>

a.py

from some_lib import some_stuff as some_stuff

use_some_suff()

The flag helps tracking that.

In any case, I can totally understand if you do not want to go through with this. As mentioned, the conf allows you to disable some checks for some modules. So feel free to close if you want to leave it at that.

@cjw296
Copy link
Member

cjw296 commented Dec 5, 2022

Well, if there had been no py.typed file in testfixtures, would mypy have complained about this?

@g-as
Copy link
Author

g-as commented Dec 5, 2022

I think so but let me test.

@g-as
Copy link
Author

g-as commented Dec 5, 2022

Tested, no py.typed, no complaints about reexports.

@cjw296
Copy link
Member

cjw296 commented Dec 5, 2022

Okay, so the solution for me is to remove the py.typed file from testfixtures :-)

@cjw296
Copy link
Member

cjw296 commented Dec 5, 2022

Removed in 42b73cf and the 7.0.4 release.

@g-as
Copy link
Author

g-as commented Dec 5, 2022

I'd rather you did not then.

@g-as
Copy link
Author

g-as commented Dec 5, 2022

LOL.

@g-as
Copy link
Author

g-as commented Dec 5, 2022

Thanks anyways.

@g-as g-as closed this as completed Dec 5, 2022
@cjw296
Copy link
Member

cjw296 commented Dec 5, 2022

Sorry, but until mypy matures or a better tool comes along that isn't written in Javascript (oh, the irony!), and the minimum Python version I want to support has this mature support, I haven't got the time or energy to appease it.

My experience has been that IDEs, certainly the JetBrains ones, will use the annotations even without the py.typed file being present and they also work as documentation, so both of my aims in having annotations present are met.

Should anyone (@asqui ?) be interested in putting a PR together which makes everything happy, and without a hard dependency on the typing-extensions package, I'd definitely try and get it merged.

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

2 participants