-
Notifications
You must be signed in to change notification settings - Fork 541
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
[0.2.0] Metadata Implementation #81
Merged
Merged
Changes from 4 commits
Commits
Show all changes
37 commits
Select commit
Hold shift + click to select a range
de083b8
Update file structure
MooooCat 7834da0
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] dd87ee1
Update single_table.py
MooooCat 14e83e6
Merge branch 'main' into feature-metadata
MooooCat 06a14f2
Update metadata (still draft)
MooooCat ac89e8e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 1c322d0
Update metadata and reset file structure
MooooCat 462b3cb
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] fb1565c
add numeric inspector
MooooCat f6da1d3
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 7e65c83
fix metadata initialization
MooooCat 329755b
fix type hits error in py38
MooooCat f2d50f5
add inspectors
MooooCat 1f4ff08
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 10f0175
Update relationship.py
MooooCat c027feb
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 9b026fc
Update multi-table-combiner
MooooCat c768a24
add composite key list in relationship
MooooCat 8af1e1d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] ed30dad
Apply suggestions from code review
MooooCat f2e4954
Apply suggestions from code review
MooooCat 44d0c61
use a single list for primary key(s)
MooooCat 75985e0
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] cfd2a11
Sync with main.
MooooCat 644568f
Update expections
MooooCat c1b239c
Update check functions
MooooCat d6077d6
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 89e4d1f
Apply suggestions from code review
MooooCat 143eeb9
Apply suggestions from code review
MooooCat 49e02c4
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] b2889da
update some test case
MooooCat 8faa6e2
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 4cf8db0
update test cases
MooooCat 98f4596
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] b50e41b
Merge branch 'main' into feature-metadata
MooooCat 7687f77
update metadata save and load test cases.
MooooCat ad312c8
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,54 @@ | ||
class MultiTableCombiner: | ||
pass | ||
from typing import Any, Dict, List | ||
|
||
from pydantic import BaseModel | ||
|
||
from sdgx.data_models.metadata import Metadata | ||
from sdgx.data_models.relationship import Relationship | ||
|
||
|
||
class MultiTableCombiner(BaseModel): | ||
"""MultiTableCombiner: combine different tables using relationship | ||
|
||
Args: | ||
metadata_dict (Dict[str, Any]): | ||
|
||
relationships (List[Any]): | ||
""" | ||
|
||
metadata_version: str = "1.0" | ||
|
||
metadata_dict: Dict[str, Any] = {} | ||
relationships: List[Any] = [] | ||
|
||
def check(self): | ||
"""Do necessary checks: | ||
|
||
- Whether number of tables corresponds to relationships. | ||
- Whether table names corresponds to the relationship between tables; | ||
""" | ||
|
||
# count check | ||
relationship_cnt = len(self.relationships) | ||
metadata_cnt = len(self.metadata_dict.keys()) | ||
if metadata_cnt != relationship_cnt + 1: | ||
raise ValueError("Number of tables should corresponds to relationships.") | ||
|
||
# table name check | ||
table_names_from_relationships = set() | ||
|
||
# each relationship's table must have metadata | ||
table_names = list(self.metadata_dict.keys()) | ||
for each_r in self.relationships: | ||
if each_r.parent_table not in table_names: | ||
raise ValueError(f"Metadata of parent table {each_r.parent_table} is missing.") | ||
if each_r.child_table not in table_names: | ||
raise ValueError(f"Metadata of child table {each_r.child_table} is missing.") | ||
table_names_from_relationships.add(each_r.parent_table) | ||
table_names_from_relationships.add(each_r.child_table) | ||
|
||
# each table in metadata must in a relationship | ||
for each_t in table_names: | ||
if each_t not in table_names_from_relationships: | ||
raise ValueError(f"Table {each_t} has not relationship.") | ||
|
||
return True |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Can we add a new exception in
sdgx/exceptions.py
? This will help us differentiate between what are expected exceptions.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 is something I didn't consider, I'll add new exceptions
MultiTableCombineError
andMetadataError
insdgx.exceptions
.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.
For better unit testing and bug fixing, I'd suggest synchronizing the main branch, which also includes
Exceptions
with error codes and exit codes