Skip to content

Add unstructuring and structuring support for deque in standard lib#355

Merged
Tinche merged 5 commits intopython-attrs:mainfrom
lqhuang:add-deque-support
May 22, 2023
Merged

Add unstructuring and structuring support for deque in standard lib#355
Tinche merged 5 commits intopython-attrs:mainfrom
lqhuang:add-deque-support

Conversation

@lqhuang
Copy link
Contributor

@lqhuang lqhuang commented Apr 19, 2023 •
edited
Loading

Env

Python==3.10.4
attrs==22.2.0
cattrs==23.1.0.dev0@98ca33ab7

What's the problem

Current head version of cattrs work normally to unstructure deque, but fails to structure obj into deque (or generic deque)

Minimal reproducible example:

@define
class C:
    x: int
    y: int

# OK
a = deque((C(x=x, y=y) for x, y in zip(range(10), range(10, 20))))
print(cattrs.unstructure(a))

# Bad
b = ({"x": x, "y": y} for x, y in zip(range(10), range(10, 20)))
print(cattrs.structure(b, deque[C]))
# Traceback (most recent call last):
#   File ".../cattrs/mycase.py", line 33, in <module>
#     main()
#   File ".../cattrs/mycase.py", line 29, in main
#     print(cattrs.structure(a, deque[C]))
#   File ".../cattrs/src/cattrs/converters.py", line 330, in structure
#     return self._structure_func.dispatch(cl)(obj, cl)
#   File ".../cattrs/src/cattrs/converters.py", line 398, in _structure_error
#     raise StructureHandlerNotFoundError(msg, type_=cl)
# cattrs.errors.StructureHandlerNotFoundError: Unsupported type: collections.deque[__main__.C]. Register a structure hook for it.

deque is a battery-included std container, so I think it's sensible to add a default structure hook for it.

Some notable facts:

print(issubclass(deque, list))
# False
print(issubclass(defaultdict, dict))
# True

I once want to also add a structure hook for defaultdict, but I thought it's probably difficult to give a factory func for defaultdict, which is better to custom by user.

What this PR does

  1. Implement function _structure_deque() and is_deque()
  2. Add related unit tests (test_structuring, test_converters)
  3. Update docs and history.

Suggestions and improvements are appreciated!

Thanks :)

@Tinche
Copy link
Member

Tinche commented Apr 19, 2023

Thanks! I'm going to PyCon soon so my attention here will be somewhat delayed, hope that's ok.

@lqhuang
Copy link
Contributor Author

lqhuang commented Apr 20, 2023

Never mind, enjoy your trip! Have fun 🤣

Looks like I need to fix some compatibility issues. I only test 3.10 on local yet.

@lqhuang
Copy link
Contributor Author

lqhuang commented Apr 20, 2023

@Tinche Updated. Could you help to let CI run again?

@codecov-commenter
Copy link

codecov-commenter commented Apr 20, 2023 •
edited
Loading

Codecov Report

Merging #355 (baee96b) into main (27e9c0d) will decrease coverage by 8.19%.
The diff coverage is 70.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main     #355      +/-   ##
==========================================
- Coverage   97.55%   89.36%   -8.19%     
==========================================
  Files          21       21              
  Lines        1675     1702      +27     
==========================================
- Hits         1634     1521     -113     
- Misses         41      181     +140
Impacted Files Coverage Δ
src/cattrs/_compat.py 59.89% <66.66%> (-37.44%) ⬇️
src/cattrs/converters.py 92.19% <70.83%> (-5.76%) ⬇️

... and 5 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@lqhuang
Copy link
Contributor Author

lqhuang commented Apr 20, 2023

@Tinche PR is ready to review. But follow your schedules, it's fine.

@Tinche Tinche self-requested a review May 18, 2023 22:54
Tinche
Tinche requested changes May 18, 2023
View reviewed changes
@lqhuang lqhuang requested a review from Tinche May 19, 2023 03:12
@Tinche Tinche added this to the 23.1 milestone May 21, 2023
Tinche
Tinche requested changes May 21, 2023
View reviewed changes
Copy link
Member

@Tinche Tinche left a comment

Choose a reason for hiding this comment

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

Left a comment, other than that, looks good!

@lqhuang
Copy link
Contributor Author

lqhuang commented May 22, 2023 •
edited
Loading

Oh. You're right! 😅 My bad

Run a real doctest

Python 3.10.11 (main, Apr  7 2023, 07:31:31) [Clang 14.0.0 (clang-1400.0.29.202)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import cattrs
>>> from collections import deque
>>> from typing import Optional
>>> cattrs.structure((1, None, 3), deque[Optional[str]])
deque(['1', None, '3'])
>>> cattrs.structure((1, 2, 3), deque[int])
deque([1, 2, 3])

@Tinche Tinche self-requested a review May 22, 2023 11:54
@Tinche
Copy link
Member

Tinche commented May 22, 2023

Thanks @lqhuang , good job. Please resolve the changelog merge conflict and let's get this in!

@lqhuang lqhuang force-pushed the add-deque-support branch from fe6ebf4 to baee96b Compare May 22, 2023 13:12
@lqhuang
Copy link
Contributor Author

lqhuang commented May 22, 2023

Already rebased from main. Thank you so much @Tinche !

@Tinche Tinche merged commit cec0035 into python-attrs:main May 22, 2023
@lqhuang lqhuang deleted the add-deque-support branch May 23, 2023 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Tinche Tinche Tinche approved these changes

Assignees

No one assigned

Labels

enhancement

Projects

None yet

Milestone

23.1

Development

Successfully merging this pull request may close these issues.

3 participants

@lqhuang @Tinche @codecov-commenter