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.

Sign up for GitHub

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

Jump to bottom

Draft: Python: Add __repr__ for Catalog #8558

Merged
merged 10 commits into from
Sep 22, 2023
Merged

Draft: Python: Add __repr__ for Catalog #8558

merged 10 commits into from
Sep 22, 2023

Conversation

jayceslesar
Copy link
Contributor

@jayceslesar jayceslesar commented Sep 13, 2023 •
edited

Similar to #8295

@jayceslesar
Copy link
Contributor Author

jayceslesar commented Sep 13, 2023 •
edited

@Fokko Traditionally __repr__ is supposed to be able to instantiate a class from exec/repr or something right? Would it make more sense to attempt to go that route and add the current work in a __str__ dunder instead? Happy to make those changes if that seems more correct.
__repr__ docs

@jayceslesar
Copy link
Contributor Author

jayceslesar commented Sep 15, 2023 •
edited

will change this to be more akin to https://github.com/apache/iceberg/pull/8447/files with just catalog

@jayceslesar jayceslesar changed the title Python: Add __repr__ for Table and Catalog Draft: Python: Add __repr__ for Catalog Sep 15, 2023
Fokko
Fokko reviewed Sep 22, 2023
View reviewed changes
Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

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

@jayceslesar This looks great, could you add a test as well?

python/pyiceberg/catalog/__init__.py Outdated Show resolved Hide resolved
@jayceslesar
Copy link
Contributor Author

@jayceslesar This looks great, could you add a test as well?

Done!

@jayceslesar @Fokko
Update python/tests/catalog/test_base.py
2b84a11 
Co-authored-by: Fokko Driesprong <fokko@apache.org>
@Fokko Fokko merged commit f2ce4ef into apache:master Sep 22, 2023
1 check passed
@Fokko
Copy link
Contributor

Fokko commented Sep 22, 2023

Thanks for fixing this @jayceslesar Much appreciated!

@jayceslesar
Copy link
Contributor Author

Thanks for fixing this @jayceslesar Much appreciated!

@Fokko Absolutely! Thanks for the review and helping gear me towards contributing more in the future

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Fokko Fokko Fokko left review comments

Assignees
No one assigned
Labels
python
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@jayceslesar @Fokko