astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
33k stars 1.1k forks source link

Rewrite `os.environ.get` as `os.getenv` #3608

Open janosh opened 1 year ago

janosh commented 1 year ago

Bad

os.environ.get(secret)
os.environ.get(secret, "fallback")

Good

os.getenv(secret)
os.getenv(secret, "fallback")
dhruvmanila commented 1 year ago

I'm a bit hesitant of this change as in the CPython implementation (https://github.com/python/cpython/blob/5e6661bce968173fa45b74fa2111098645ff609c/Lib/os.py#L779-L783), they both are equivalent and there's no set recommendation to use one or the other.

However, there is a difference between os.environ[key] and os.getenv(key) where the former will raise a KeyError if the key doesn't exist in the environment.

Another thing to note is that in the documentation, there's this note regarding updating an environment variable:

Note Calling putenv() directly does not change os.environ, so it’s better to modify os.environ.

Ref: https://docs.python.org/3/library/os.html#os.environ

janosh commented 1 year ago

Just a suggestion. Feel free to close or put this in one of the opinionated rule sets.

Tatsh commented 11 months ago

I would actually recommend the opposite. putenv does not update os.environ. getenv caches os.environ at import time which can happen at the very first import in any given module.

Note Calling putenv() directly does not change os.environ, so it’s better to modify os.environ.

Note that since getenv() uses os.environ, the mapping of getenv() is similarly also captured on import, and the function may not reflect future environment changes.

I was searching if there was a rule to error when getenv/putenv are encountered to replace with os.environ. That would be a welcome addition and sadly I don't know enough Rust.

Vir2S commented 16 hours ago

I'm a bit hesitant of this change as in the CPython implementation (https://github.com/python/cpython/blob/5e6661bce968173fa45b74fa2111098645ff609c/Lib/os.py#L779-L783), they both are equivalent and there's no set recommendation to use one or the other.

However, there is a difference between os.environ[key] and os.getenv(key) where the former will raise a KeyError if the key doesn't exist in the environment.

Another thing to note is that in the documentation, there's this note regarding updating an environment variable:

Note Calling putenv() directly does not change os.environ, so it’s better to modify os.environ. Ref: https://docs.python.org/3/library/os.html#os.environ

They are not totally equivalent)