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

Add environment defaults and add docs #5

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ropottnik
Copy link

closes #4

@Jeremy-Stafford if you decide to merge this PR, could you then also make a new release on pypi? Thanks.

@ropottnik
Copy link
Author

@Jeremy-Stafford could you have a look?

@Jeremy-Stafford
Copy link
Owner

Sorry for the rather slow response, GitHub was supposed to send me an email notification, but did not.

I would suggest the following changes:

  • Improve formatting of doc-strings: there should be a summary of each function on the line with the """, then a double line break, then any extra details, preferably in paragraphs.
  • Put all the imports together, after the module-level docstring.
  • I think has_env_default_factory and get_corresponding_env_var are unecessary and that using default_factory directly is a better way to go. Something like
import os
from typing import *

def from_env_var(name: str) -> Function[str]:
     return lambda: os.getenv(name)

Perhaps with some extra attributes for the lambda.

I currently don't have access to a computer (for about 1 week), so I can't write code very well or update the package on PyPI.

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

Successfully merging this pull request may close these issues.

add defaults for environment variables and update docs
2 participants