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

How do I get pylance to recognize the _ ("underscore") function from gettext. #1383

Closed
lucaspcamargo opened this issue Jun 2, 2021 · 21 comments
Labels
fixed in next version (main) A fix has been implemented and will appear in an upcoming version needs investigation Could be an issue - needs investigation

Comments

@lucaspcamargo
Copy link

Hello, all
I work on a project that uses GNU gettext for translating strings. I tried to use the "typings" functionality to get it to recognize the "_" function as follows:

# somefile.pyi
def _(in: str) -> str: ...
def coolstub(in:str) -> str: ...

coolstub(...) is recognized and available globally. Is there a way to make pylance recognize _(...) as well?

Thanks in advance :)

@github-actions github-actions bot added the triage label Jun 2, 2021
@jakebailey
Copy link
Member

Are you just seeing that _ isn't available externally? Symbols that begin with _ are treated as private to the module, which may be why you're not seeing them externally (although we may also be treating the symbol _ specially, though it is a valid symbol name). What does the code that uses this function look like? Are you explicitly importing _, or are you relying on a * import?

If you define an __all__ that contains _, does it work as you expect? E.g.:

# stub contents ...

__all__ = ['_', 'coolstub']

@lucaspcamargo
Copy link
Author

lucaspcamargo commented Jun 2, 2021

Unfortunately, it does not.
I thought it would have something to do with underscore being private by default, and I'd already tried exporting the symbol explicitly, to no avail. No symbols starting with '_' get recognized, even if exported explicitly. Maybe there is something else at work, or the __all__ mechanism does not apply to pyi files?

@jakebailey
Copy link
Member

__all__ does apply to pyi files.

This seems more like our analysis is making some assumptions about _ itself, which is typically used as a discard variable and not an actual usable value (beside in the REPL, where it always means the previous evaluation).

@lucaspcamargo
Copy link
Author

Alright, I think I understand the issue better.
It'd be nice if Pylance could support such a usecase, as in "advanced symbols are being injected into the global namespace, and the developer wants to annotate that".
Thank you so much for the lightning-quick response, I'm impressed!

@jakebailey
Copy link
Member

I guess the thing I'm confused about is how you were using a stub to add things into a global namespace; as far as I'm aware, the only ways to do so are:

  • Fork typeshed, add what you need to builtins.pyi.
  • Create a dummy module that doesn't exist at runtime and * import it in a if TYPE_CHECKING block.
  • "Declare" them in a if TYPE_CHECKING block with their types.

You can't have somefile.pyi, then go to somefile.py and see the types you declared there; the stubs are the external API of a module to other files and do not affect the "real source" they are describing (#1238).

@lucaspcamargo
Copy link
Author

lucaspcamargo commented Jun 2, 2021

I'm sorry, I wasn't truly adding things to the global namespace.
There was autocompletion of the 'coolstub' symbol coming from somewhere, so I thought that was working, but now, if I try to reference it, Pylance asks me to import it from a module named the same as my pyi file, or from the 'typings' module, if my pyi file is named __init__.pyi.
So yeah, I understand now that any file I put on the 'typings' folder will be understood as describing a module, and would not ever do anything to the built-ins namespace in the first place. My bad.
The ways you have outlined above all make sense. Since I want to get it working without modifying my source, I tend towards the first solution. If I fork typeshed on my interpreter's virtual environment, would that be the instance/installation used by Pylance?

@jakebailey
Copy link
Member

It's not really a good option to fork typeshed; how are the symbols appearing? Are they imported from somewhere, or are they getting injected by some wrapper?

@lucaspcamargo
Copy link
Author

lucaspcamargo commented Jun 2, 2021

It is showing up as such:

Screenshot_20210602_173400

inside the '${workspace}/typings' folder I have an __init__.pyi file with the following contents:

def _(in: str) -> str: ...
def coolstub(in: str) -> str: ...

__all__ = ['_', 'coolstub']

I had named it __init__.pyi to see if would show up in the global namespace. Yeah, now I know that's not going to happen.
On the other hand, if I name it something.pyi, nothing changes, except that Auto-import will ask if I want to import the symbol from the something module.

Before, It was showing up on autocomplete with a blue icon (little box within brackets). Can't reproduce that anymore.

@jakebailey
Copy link
Member

Yeah, you don't want to have an __init__.pyi in typings. That would be some magic module without a name, as typings is effectively an import root. For example, if you wanted to make a stub for somelibrary, you'd have typings/somelibrary/__init__.pyi.

Having the the __init__.pyi at the root confuses things, as it then thinks that typings is an importable module, which it isn't. No __init__ in a search path is importable that way, so that's probably a case we should fix to avoid confusion, but it's not the way to add stuff to the builtins.

@judej judej added needs decision Do we want this enhancement? needs investigation Could be an issue - needs investigation labels Jun 3, 2021
@github-actions github-actions bot removed the triage label Jun 3, 2021
@savannahostrowski savannahostrowski added waiting for user response Requires more information from user needs investigation Could be an issue - needs investigation and removed needs decision Do we want this enhancement? needs investigation Could be an issue - needs investigation waiting for user response Requires more information from user labels Jun 8, 2021
@lucaspcamargo
Copy link
Author

lucaspcamargo commented Jun 9, 2021

I see that the "waiting for user response" label was added to this ticket, but I'm not sure what more information I could provide. From what I got from our discussion, the usage scenario was explained and understood, but it is not yet achievable with Pylance. I'm eager to provide any additional information that is required.

@jakebailey
Copy link
Member

No, that was done in error; it's back to being marked as needing investigation.

@MightyCreak
Copy link

I think I'm also hit by that bug. I'm not sure because it went quite fast to workarounds in __init__.pyi file and such.

In my case I simply use gettext and its gettext.install() function, which declares the _() function. I have several hundreds of "_" is not defined messages.

Here's my project: https://github.com/MightyCreak/diffuse

@erictraut
Copy link
Contributor

It appears that the gettext library is dynamically injecting symbols into the builtins namespace. Here's the code that does this:

builtins.__dict__['_'] = self.gettext

This is highly suspect and inadvisable. Libraries shouldn't be overwriting the namespace of any other module, especially stdlib modules and most especially the builtins module! This practice can result in library namespace collisions and difficult-to-debug initialization ordering bugs. I'm quite surprised that a library would contain such hacky behavior.

The good news is that this library doesn't automatically do this namespace modification upon initialization. You will get this undesirable behavior only if you call gettext.install(). I recommend not calling that method.

My recommendation is that you replace the call to gettext.install() with the following:

    import gettext
    translator = gettext.translation('diffuse', LOCALEDIR)
    _ = translator.gettext

You could add this code to each file that uses the _ function or place it in a common utility module if you prefer to allocate only a single translation object for your entire project. Other modules in your project could then explicitly import _ from the utility module.

You might also want to consider giving the symbol a more appropriate name since _ is not self-documenting and is conventionally used in Python to indicate "a symbol whose value I don't care about".

@MightyCreak
Copy link

I understand the arguments, but AFAIK (and I might be wrong), gettext is a well-known and widely-used lib, and using _() around your translated string is a known pattern in several languages.

@kthy
Copy link

kthy commented Feb 2, 2022

gettext isn't just widely known, it's in the python stdlib itself, and as per the documentation it conventionally relies on injecting the _() function into the builtins. This is then used by utilities like GNU gettext or Babel to extract the necessary strings for i18n.

In flake8 we're able to specify

builtins = _

to add _ to the list of builtins. Any chance of a similar configuration option here?

@lucaspcamargo
Copy link
Author

lucaspcamargo commented Feb 2, 2022

For sure that injecting functions into the builtin namespace isn't "good practice", but it's a very common pattern for translation libraries, in one way or another, for several languages. The suggested alternative of manually declaring or importing _ wherever it's used is highly undesirable, because:

  • You can't really do that to large, established codebases; Well, you can, but in reality, in many production environments, you cannot.
  • The reason that _ is usually injected as a global symbol is because translations are supposed to be ubiquitous, especially in user-interface code.
  • Having to add repeated boilerplate to most files in a project just can't be good practice, can it?

IMO, what we need is a mechanism for us to tell pylance that such and such is in the builtin namespace. I can't see any other reasonable way of supporting gettext users.

@erictraut
Copy link
Contributor

I investigated various ways we could support this. The solution I landed on was a special type stub file __builtins__.pyi. If pyright or pylance find a type stub file with this name, it adds the symbols declared in this stub file to the builtins scope. I recommend placing this stub file in your root project directory or within the subdirectory referenced by the python.analysis.stubPath setting (by default, it's called typings). For more details, refer to this documentation. This functionality will be included in the next release of pyright and pylance.

@erictraut erictraut added fixed in next version (main) A fix has been implemented and will appear in an upcoming version and removed in backlog labels Feb 3, 2022
@kthy
Copy link

kthy commented Feb 3, 2022

Just to check if I understand the format right, the contents of __builtins__.pyi should essentially be

def _(__msg: str) -> str: ...

right?

@erictraut
Copy link
Contributor

Yes, that's the correct syntax. I don't know if that's the right signature for your use case.

You could optionally use a positional-only separator rather than using the old-style "double underscore" convention.

def _(msg: str, /) -> str: ...

@lucaspcamargo
Copy link
Author

Thank you so much, Eric! I'll give a spin as soon as the next version comes out.

@debonte
Copy link
Contributor

debonte commented Feb 10, 2022

This issue has been fixed in version 2022.2.1, which we've just released. You can find the changelog here: CHANGELOG.md

@debonte debonte closed this as completed Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed in next version (main) A fix has been implemented and will appear in an upcoming version needs investigation Could be an issue - needs investigation
Projects
None yet
Development

No branches or pull requests

8 participants