Skip to content
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

Centralize root_tables function #1934

Open
gsheni opened this issue Apr 18, 2024 · 0 comments
Open

Centralize root_tables function #1934

gsheni opened this issue Apr 18, 2024 · 0 comments
Labels
internal The issue doesn't change the API or functionality

Comments

@gsheni
Copy link
Contributor

gsheni commented Apr 18, 2024

Description

  • There is currently 2 places we get the root parents:
  • SDV/sdv/_utils.py

    Lines 363 to 366 in 447d0d0

    def _get_root_tables(relationships):
    parent_tables = {rel['parent_table_name'] for rel in relationships}
    child_tables = {rel['child_table_name'] for rel in relationships}
    return parent_tables - child_tables
  • SDV/sdv/multi_table/base.py

    Lines 117 to 122 in 938bea0

    def _get_root_parents(self):
    """Get the set of root parents in the graph."""
    non_root_tables = set(self.metadata._get_parent_map().keys())
    root_parents = set(self.metadata.tables.keys()) - non_root_tables
    return root_parents
  • Ideally, this code lives in one place, most likely as a private method on the MultiMetadata class.
@gsheni gsheni added the new Automatic label applied to new issues label Apr 18, 2024
@npatki npatki added internal The issue doesn't change the API or functionality and removed new Automatic label applied to new issues labels Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal The issue doesn't change the API or functionality
Projects
None yet
Development

No branches or pull requests

2 participants