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

Use metadata over singletablemetadata #2144

Closed

Conversation

lajohn4747
Copy link
Contributor

resolves #2128
CU-86b19amxa

Made metadata work with single table synthesizers by converting SingleTableMetadata to Metadata before inserting into the synthesizers. Write a wrapper method so that all single table synthesizers can use Metadata when there is only one table.

Wrote unit tests and update tests to use the methods instead of directly accessing SingleTableMetadata attributes.

@sdv-team
Copy link
Contributor

@lajohn4747 lajohn4747 changed the title Issue 2128 use metadata over single Issue 2128 use metadata over singletablemetadata Jul 23, 2024
@lajohn4747 lajohn4747 changed the base branch from main to feature/metadata July 23, 2024 19:38
sdv/lite/single_table.py Outdated Show resolved Hide resolved
sdv/metadata/metadata.py Show resolved Hide resolved
sdv/metadata/metadata.py Outdated Show resolved Hide resolved
sdv/metadata/metadata.py Outdated Show resolved Hide resolved
sdv/metadata/metadata.py Outdated Show resolved Hide resolved
sdv/metadata/metadata.py Show resolved Hide resolved
sdv/metadata/metadata.py Outdated Show resolved Hide resolved
sdv/metadata/metadata.py Outdated Show resolved Hide resolved
sdv/metadata/metadata.py Outdated Show resolved Hide resolved
sdv/metadata/metadata.py Outdated Show resolved Hide resolved
@@ -254,7 +254,7 @@ def _validate_metadata_specific_to_constraint(metadata, **kwargs):
invalid_columns = []
column_names = kwargs.get('column_names')
for column in column_names:
if metadata.columns[column]['sdtype'] not in ['boolean', 'categorical']:
if metadata.get_columns()[column]['sdtype'] not in ['boolean', 'categorical']:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't be backwards compatible. SingleTableMetadata does not have get_columns().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made it backwards compatible by making sure all metadata is converted and also convert it the synthesizer metadata on load.

Also added an integration test for backward compatibility.

@lajohn4747 lajohn4747 changed the title Issue 2128 use metadata over singletablemetadata Use metadata over singletablemetadata Jul 25, 2024
sdv/lite/single_table.py Outdated Show resolved Hide resolved
tests/integration/sequential/test_par.py Show resolved Hide resolved
tests/integration/metadata/test_metadata.py Outdated Show resolved Hide resolved
tests/integration/metadata/test_metadata.py Show resolved Hide resolved
Copy link
Contributor

@frances-h frances-h left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 👍

Co-authored-by: gsheni <gvsheni@gmail.com>
@lajohn4747
Copy link
Contributor Author

Closing as we landed #2140 instead

@lajohn4747 lajohn4747 closed this Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable single table synthesizers to use new Metadata
6 participants