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

Improve usage of detect_from_dataframes function #2221

Merged
merged 6 commits into from
Sep 12, 2024

Conversation

amontanez24
Copy link
Contributor

resolve #2214
CU-86b23k3j9

@amontanez24 amontanez24 requested a review from a team as a code owner September 11, 2024 16:58
@amontanez24 amontanez24 requested review from frances-h and removed request for a team September 11, 2024 16:58
@sdv-team
Copy link
Contributor

@amontanez24 amontanez24 changed the base branch from main to feature/metadata September 11, 2024 16:58
Copy link
Contributor

@lajohn4747 lajohn4747 left a comment

Choose a reason for hiding this comment

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

LGTM

@amontanez24 amontanez24 changed the title Issue 2214 detect from dataframes Improve usage of detect_from_dataframes function Sep 11, 2024
@amontanez24 amontanez24 force-pushed the issue-2214-detect-from-dataframes branch from 048ec5a to cef7ec4 Compare September 12, 2024 16:39
@amontanez24 amontanez24 requested review from rwedge and removed request for frances-h September 12, 2024 18:11
Copy link
Contributor

@rwedge rwedge left a comment

Choose a reason for hiding this comment

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

Should any changes be made to MultiTableMetadata.detect_from_dataframes?

data = {'guests': guests_table, 'hotels': hotels_table}

# Run
Metadata.detect_from_dataframes(data)
Copy link
Contributor

Choose a reason for hiding this comment

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

should we also test the returned value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, good catch!

@amontanez24 amontanez24 merged commit b1a7f7d into feature/metadata Sep 12, 2024
39 checks passed
@amontanez24 amontanez24 deleted the issue-2214-detect-from-dataframes branch September 12, 2024 21: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.

Improve usage of detect_from_dataframes function
4 participants