-
Notifications
You must be signed in to change notification settings - Fork 25
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
Improve dosctrings for DataSources #1808
base: master
Are you sure you want to change the base?
Conversation
8b85394
to
765f41d
Compare
5fd4060
to
8c05453
Compare
…or' into api/api-ref-data-sources_fix_error # Conflicts: # .github/workflows/ci.yml
…or' into api/api-ref-data-sources_fix_error
d283d5a
to
3eb4157
Compare
key: Optional[str] = "", | ||
is_domain: Optional[bool] = False, | ||
domain_id: Optional[int] = 0, |
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.
I do not think you have to use Optional
here as giving them a default value makes them optional already. Plus, using Optional
technically means it accepts None
as a value, right? Which is not the case here.
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.
I haven't checked previous code for that in particular but it is worth checking for, because otherwise the user can call these with None
and it will not work.
key: Optional[str] = "", | ||
result_key: Optional[str] = "", |
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.
Again for Optional
def add_upstream(self, upstream_data_sources, result_key=""): | ||
"""Add upstream data sources. | ||
def add_upstream( | ||
self, upstream_data_sources: DataSources, result_key: Optional[str] = "" |
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.
self, upstream_data_sources: DataSources, result_key: Optional[str] = "" | |
self, upstream_data_sources: DataSources, result_key: str = "" |
>>> | ||
>>> # Get the path to the main result files of the DataSources object | ||
>>> my_data_sources.result_files | ||
[None, None] |
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.
@luisaFelixSalles that actually looks like a bug?
Co-authored-by: Paul Profizi <[email protected]>
Co-authored-by: Paul Profizi <[email protected]>
Co-authored-by: Paul Profizi <[email protected]>
Co-authored-by: Paul Profizi <[email protected]>
Co-authored-by: Paul Profizi <[email protected]>
Co-authored-by: Paul Profizi <[email protected]>
Co-authored-by: Paul Profizi <[email protected]>
Co-authored-by: Paul Profizi <[email protected]>
Co-authored-by: Paul Profizi <[email protected]>
Co-authored-by: Paul Profizi <[email protected]>
Co-authored-by: Paul Profizi <[email protected]>
Co-authored-by: Paul Profizi <[email protected]>
No description provided.