-
Notifications
You must be signed in to change notification settings - Fork 2
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
Multiple small fixes to docstrings and typing #282
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good changes overall. I think the Optional
usage needs to be clarified here. Will be good to go afterwards!
EDIT: To reiterate, the Optional
serves both the IDE/user and helps with the static type checking that would come from mypy. We don't currently have any projects that are 100% mypy
-compliant, but the more we implement now, the easier it will be in the future.
Why use mypy
? https://python.plainenglish.io/mypy-your-secret-weapon-for-code-brilliance-a376d15c463c
Co-authored-by: juliettelavoie <[email protected]>
Co-authored-by: juliettelavoie <[email protected]>
Co-authored-by: Trevor James Smith <[email protected]>
Co-authored-by: Trevor James Smith <[email protected]>
Co-authored-by: Trevor James Smith <[email protected]>
Co-authored-by: Trevor James Smith <[email protected]>
# Conflicts: # HISTORY.rst
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lots of work here. looking good!
Pull Request Checklist:
number
) and pull request (:pull:number
) has been added.What kind of change does this PR introduce?
str
andos.PathLike
should be explicitly supported where relevant.Does this PR introduce a breaking change?
Other information:
ABC
toDataCatalog/ProjectCatalog
. It this right? @aulemahalinit
and documentation ofDataCatalog/ProjectCatalog
should be reworked to be easier to understand, but this is out-of-scope for this PR. For example we use*args
, but AFAIK there is only one possible input argument,obj
.