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

Make Env methods self-documenting #186

Closed
tomgrin10 opened this issue Nov 2, 2020 · 5 comments
Closed

Make Env methods self-documenting #186

tomgrin10 opened this issue Nov 2, 2020 · 5 comments

Comments

@tomgrin10
Copy link
Contributor

I would like to preface this issue by saying that I really love this library, and use it a lot in my projects 😀

I personally see great importance in self-documenting code, and a large part of self-documenting code is being able to view
a function's arguments and their types in an IDE when using an API, without looking at its docs.
As this library has no official API reference (which is totally fine!), I think its crucial that the function signatures are clear to the user.

I see 2 choices for achieving this:

  1. Adding a .pyi file. (this means that every change to Env's API means a change in the .pyi file too)
  2. Rewrite Env's methods so that they have a clear signature that IDEs can parse.

I personally think the 1st option is easier, but wanted to know your opinion before submitting a PR.

@sloria
Copy link
Owner

sloria commented Nov 2, 2020

I would like to preface this issue by saying that I really love this library, and use it a lot in my projects 😀

Thanks for the kind words

a large part of self-documenting code is being able to view
a function's arguments and their types in an IDE when using an API, without looking at its docs.
As this library has no official API reference (which is totally fine!), I think its crucial that the function signatures are clear to the user.

I suppose I opted to prioritize conciseness and ease of maintenance over static analyzability and self-documentation (related issue: #120). That said, I'd be willing to budge if your proposal can be done without too much maintenance overhead. So feel free to send a PR. A rough proof of concept should suffice.

@tomgrin10 tomgrin10 changed the title Make Env methods self-documenting Make Env methods self-documenting Nov 2, 2020
@hukkin
Copy link

hukkin commented Nov 6, 2020

Hey, chipping in since I did quite a lot of research on this in #120 😃

I personally think the 1st option is easier...

Getting the typings right might not be as easy as you initially think (without making any functional changes to the codebase). #120 describes the problem points quite extensively. Neglecting the eager=False feature the annotations will be fairly complex (I made a package with reduced API, e.g. no eager=False, you can see the monstrous type annotations there 😄 ). Respecting the eager=False feature, it seems the equation is pretty much impossible for a type checker to solve.

Note that distributing inaccurate typings that neglect a certain use case is probably more annoying to users than typing everything as typing.Any.

@mfwarren
Copy link

mfwarren commented Nov 6, 2020

starting with 9.0.0 mypy complains about this example:

from environs import Env
env = Env(eager=True)

ENV_VALUE = env('ENV_VALUE')
ENV_VALUE.startswith('string')

is there a suggestion for an elegant way to handle types in this case?

@hukkin
Copy link

hukkin commented Nov 6, 2020

@mfwarren I would say the type annotations need to be loosened here. I made a PR for you #188

Meanwhile you can fix mypy complaining by adding assert isinstance(ENV_VALUE, str) immediately after reading the value. Or alternatively revert to a previous environs release.

@sloria
Copy link
Owner

sloria commented Jan 9, 2024

Closing this for now as i don't think there's any immediate action to take. We can always reopen if people want to discuss further

@sloria sloria closed this as completed Jan 9, 2024
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

4 participants