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

Issue 2130 metadata for evaluation #2149

Closed
wants to merge 26 commits into from

Conversation

@lajohn4747 lajohn4747 requested a review from a team as a code owner July 25, 2024 20:48
@sdv-team
Copy link
Contributor

Base automatically changed from issue_2129_metadata_over_multi to issue_2128_use_metadata_over_single August 1, 2024 14:55
@pvk-developer pvk-developer requested review from gsheni and removed request for a team, frances-h and pvk-developer August 2, 2024 16:55
if isinstance(metadata, SingleTableMetadata):
self.metadata = Metadata().load_from_dict(metadata.to_dict())
warnings.warn(DEPRECATION_MSG, FutureWarning)
self.metadata = Metadata()._convert_to_unified_metadata(metadata)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need to rebase this. We're not converting to unified metadata here anymore

Copy link
Member

Choose a reason for hiding this comment

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

I think we need to change the base of this, this is going to the 2128 branch but I think it has to go to feature/metadata 🤦🏻‍♂️

if len(metadata.tables) > 1:
raise ValueError('Only a single table is allowed in metadata.')

metadata_dict = next(iter(metadata.tables.values())).to_dict()
Copy link
Contributor

Choose a reason for hiding this comment

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

In the case where it is the old metadata, we convert it to the new one by converting to a cit, and then loading from that. Then we go back to a dict here. Maybe we should just convert to a dict and raise the warning instead of converting back and forth

@pvk-developer pvk-developer changed the base branch from issue_2128_use_metadata_over_single to feature/metadata August 5, 2024 11:34
@lajohn4747
Copy link
Contributor Author

Closing this pull request as we went with a direction: #2174

@lajohn4747 lajohn4747 closed this Aug 8, 2024
@pvk-developer pvk-developer deleted the issue_2130_metadata_for_evaluation branch October 10, 2024 12:03
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.

5 participants