-
Couldn't load subscription status.
- Fork 72
[SYNPY-1653] Create RecordSet, Grid, and CurationTask classes #1246
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
[SYNPY-1653] Create RecordSet, Grid, and CurationTask classes #1246
Conversation
…n creation documentation
| return self | ||
| else: | ||
| if not self.project_id: | ||
| raise ValueError("project_id is required to create a CurationTask") |
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.
This pattern pops up a lot in the various model classes. I wonder if this should be abstracted out into something that could be used as a function/decorator:
def required_attribute decorator(req_attributes: list[str]) -> None:
for att in req_attributes:
val = getattr(self, att)
if val is None:
raise ValueError(f"{att} is required for this method")
|
As a note tests are patched in #1256 |
| """Summary statistics from the export response""" | ||
|
|
||
| def fill_from_dict( | ||
| self, synapse_response: Union[Dict[str, Any], Any] |
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.
Nit: This type is redudant: Union[Dict[str, Any], Any] could replace with just Any
|
LGTM, Really exciting to see all the OOP Models! |
… skip integration tests for code that will be removed
Actually covers SYNPY-1653 and SYNPY-1655
Problem:
Solution:
Testing:
Unit tests
Integration tests
Did manual testing during development, but needs more comprehensive testing