-
Notifications
You must be signed in to change notification settings - Fork 423
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
Enable xattr
test for macOS
#4845
Conversation
tests/conftest.py
Outdated
from collections import defaultdict | ||
from pathlib import Path | ||
from typing import Generator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deprecated since version 3.9: collections.abc.Generator now supports subscripting ([]). See PEP 585 and Generic Alias Type (source: https://docs.python.org/3/library/typing.html?highlight=typing#typing.Generator)
You can also just use Iterator
too if you don't need to specify the extra types in a Generator
:
from collections.abc import Iterator
def oh_it_iterates() -> Iterator[str]:
yield "iter"
yield "ator"
yield "!!!"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh cool! I didn't know this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, since this was a change introduced in Python 3.9 we'd have to do a try-except to use the updated subscriptable types. So I think we should wait until we drop Python 3.8 support before making the switch from typing
to collections.abc
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I just want to make sure we are not using any of the deprecated types in the typing
module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
Description
Xattr test previously only ran on Linux (if
setfattr
is present), made necessary changes to allow the test to also run on macOS.Updated
testing_homedir
fixture to use a simplertempfile.TemporaryDirectory
implementation.Checklist - did you ...
news
directory (using the template) for the next release's release notes?Add / update outdated documentation?