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

Fix: Add custom repr to pretty print collections.UserList #13320

Merged
merged 3 commits into from Dec 2, 2021

Conversation

007vedant
Copy link
Contributor

Implemented _userlist_pprint to enable pretty printing of collections.UserList.
Followed convention of previously implemented pprints of other container types of the collections module.
solved against issue #13283

Eg Usage:

In [3]: collections.UserList(range(5))
Out[3]: UserList([0, 1, 2, 3, 4])

In [4]: collections.UserList(range(50))
Out[4]: 
UserList([0,
          1,
          2,
          3,
          4,
         ...
          46,
          47,
          48,
          49])

if cycle:
p.pretty(cls_ctor(RawText("...")))
else:
p.pretty(cls_ctor(list(obj)))
Copy link

Choose a reason for hiding this comment

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

Does this meant we recreate the list rather than accessing UserList.data?

Copy link
Member

Choose a reason for hiding this comment

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

It might be a small optimisation, not sure it is worth it, though the current implementation is identical to _deque_pprint, so we should either reuse _deque_pprint or use obj.data. Up to you.

  • a test would indeed be good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Carreau Thanks for the input. I'll start working on it.

Copy link
Contributor Author

@007vedant 007vedant Nov 27, 2021

Choose a reason for hiding this comment

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

@Carreau I have updated _userlist_pprint with obj.data for optimization and added test for the same in lib/tests/test_pretty.py. All tests have passed. ✔️

Please Review.

@yurzo
Copy link

yurzo commented Nov 24, 2021

Is this tested anywhere in regression?

if cycle:
p.pretty(cls_ctor(RawText("...")))
else:
p.pretty(cls_ctor(list(obj)))
Copy link
Member

Choose a reason for hiding this comment

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

It might be a small optimisation, not sure it is worth it, though the current implementation is identical to _deque_pprint, so we should either reuse _deque_pprint or use obj.data. Up to you.

  • a test would indeed be good.

… lists.

 -implemented _userlist_pprint , in accordance with pprint of other container types of collections module.
@Carreau
Copy link
Member

Carreau commented Dec 1, 2021

Do you have an autoformatter setup on your editor by any chance ?

rebased/cleaned and force pushed from 63e98d9 to aca50d2

@Carreau Carreau added this to the 8.0 milestone Dec 2, 2021
@Carreau Carreau merged commit 1de6ce7 into ipython:master Dec 2, 2021
@Carreau Carreau linked an issue Jan 12, 2022 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

collections.UserList does not get the same love as list
3 participants