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

Implement datatyping.printer with subclassing #7

Open
carlbordum opened this issue Oct 1, 2018 · 5 comments
Open

Implement datatyping.printer with subclassing #7

carlbordum opened this issue Oct 1, 2018 · 5 comments

Comments

@carlbordum
Copy link
Owner

I was super stupid, when I implemented datatyping.printer.py. It should be implemented with subclassing instead of monkey patching (with context managers). This would also make it thread-safe.

Relevant: #2.

@dmonego
Copy link

dmonego commented Oct 1, 2018

Hey! I just started to do some work on this.

One thing I noticed is that _test_printer.py is hidden from the test suite. Does that have an existing test failure that I should try to fix?

dmonego pushed a commit to dmonego/datatyping that referenced this issue Oct 1, 2018
@dmonego
Copy link

dmonego commented Oct 1, 2018

This PR has the change: #9

@carlbordum
Copy link
Owner Author

This is super nice!

Does it make sense for datatyping.printer._new_safe_repr to be in the subclass aswell?

I think I disabled the test, because it makes a http request. Maybe once #4 is done, we can have tests for this.

@dmonego
Copy link

dmonego commented Oct 2, 2018

I don't think _new_safe_repr should be in the subclass - I've tried to put the minimum number of overrides in there, and the current way mirrors how pprint._safe_repr works.

@carlbordum
Copy link
Owner Author

Awesome work!

/merged/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants