Miksus / rocketry

Modern scheduling library for Python
https://rocketry.readthedocs.io
MIT License
3.27k stars 106 forks source link

fix: EnvVar default type allows str #176

Closed tekumara closed 1 year ago

tekumara commented 1 year ago

resolves #173

Miksus commented 1 year ago

Indeed, the EnvArg can only have str (or not set) as the default value. Thanks for fixing this!

Been a bit lazy recently and currently got lost with adding API to Rocketry. It will take maybe till Wednesday to check this (afraid to merge as being this tired even though I don't see anything wrong).

tekumara commented 1 year ago

No problem!

BTW, what do you think about using an Optional type and None here instead of NOTSET, eg:

  def __init__(self, key:Any, default:Optional[str]=None):

It would be more idiomatic.

Miksus commented 1 year ago

Sorry, have had a bit of a break (been too busy with hobbies) and I hope I can now return to develop Rocketry more frequently.

I'll try tomorrow come back to this. Sorry for the wait.

Miksus commented 1 year ago

Yep, I agree with those changes and looks good! Thanks for adding the type hints!