-
Notifications
You must be signed in to change notification settings - Fork 25
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
[python] Python 3.12 support #3001
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -783,7 +783,9 @@ def make_multiply_indexed_dataframe( | |
"domain": [[-1000, 1000]], | ||
"coords": [{"bogus": True}], | ||
"A": None, | ||
"throws": TypeError, | ||
# Disable Typeguard while asserting this error, otherwise a typeguard.TypeCheckError is | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's hvae a GitHub issue tracking the 'real' typeguard release, and let's put its URL here as a to-do comment There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change isn't due to using Typeguard@HEAD; it will remain relevant when we move back to a proper Typeguard release. The reason for it is: in 3.12, better type-checking was causing a I worked around it by disabling type-checking while triggering this error, so we can continue to verify what we were previously. |
||
# raised (though that's not what would happen in production) | ||
"throws": (TypeError, False), | ||
}, | ||
{ | ||
"name": "bad index type bool", | ||
|
@@ -890,15 +892,30 @@ def test_read_indexing(tmp_path, io): | |
read_kwargs.update( | ||
{k: io[k] for k in ("coords", "partitions", "value_filter") if k in io} | ||
) | ||
if io.get("throws", None): | ||
with pytest.raises(io["throws"]): | ||
|
||
# `throws` can be `Type[Exception]`, or `(Type[Exception], bool)` indicating explicitly | ||
# whether Typeguard should be enabled during the `with raises` check. | ||
throws = io.get("throws", None) | ||
if throws: | ||
if isinstance(throws, tuple) and not throws[1]: | ||
# Disable Typeguard, verify actual runtime error type (avoid | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto the above to-do URL/comment There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above, todo related to upgrading Typeguard is not applicable here. |
||
# `typeguard.TypeCheckError` short-circuit) | ||
throws = throws[0] | ||
throws_ctx = raises_no_typeguard | ||
else: | ||
throws_ctx = pytest.raises | ||
else: | ||
throws_ctx = None | ||
|
||
if throws_ctx: | ||
with throws_ctx(throws): | ||
next(sdf.read(**read_kwargs)) | ||
else: | ||
table = next(sdf.read(**read_kwargs)) | ||
assert table["A"].to_pylist() == io["A"] | ||
|
||
if io.get("throws", None): | ||
with pytest.raises(io["throws"]): | ||
if throws_ctx: | ||
with throws_ctx(throws): | ||
next(sdf.read(**read_kwargs)).to_pandas() | ||
else: | ||
table = next(sdf.read(**read_kwargs)).to_pandas() | ||
|
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.
This would be the place for a TODO / issue link related to upgrading Typeguard. Do you want me to change this comment?
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.
Filed #3216 and referenced it here