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.

Sign up for GitHub

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

Jump to bottom

[pytx] fix broken gh actions for pytest and lint #1254

Merged
merged 3 commits into from
Jan 19, 2023
Merged

[pytx] fix broken gh actions for pytest and lint #1254

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
01f414f
use max of signed int to avoid interger overflow in set of test
BarrettOlson Jan 19, 2023
7de3538
attempt to update type error and pin mypy version
BarrettOlson Jan 19, 2023
879ff1e
black format
BarrettOlson Jan 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion python-threatexchange/setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
extras_require["test"] = sorted({"pytest"} | all_extras)
extras_require["package"] = ["wheel"]
extras_require["lint"] = ["black"]
extras_require["types"] = ["mypy", "types-python-dateutil", "types-requests"]
extras_require["types"] = ["mypy==0.991", "types-python-dateutil", "types-requests"]
extras_require["all"] = sorted(set(sum(extras_require.values(), [])))
# If you are developing pytx, use this install
# Note that without ffmpeg (for vpdq) you may get errors still
Expand Down
4 changes: 2 additions & 2 deletions python-threatexchange/threatexchange/cli/tests/e2e_test_helper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def cli_call(self, *given_args: str) -> str:
inner_main(args, state_dir=self._state_dir)
except SystemExit as se:
if se.code != 0:
raise E2ETestSystemExit(se.code)
raise E2ETestSystemExit(t.cast(int, se.code))
return fake_out.getvalue()

def assert_cli_output(
Expand All @@ -72,7 +72,7 @@ def assert_cli_output(
assert lines[line] == expected_line_output

def assert_cli_usage_error(
self, args: t.Iterable[str], msg_regex: str = None
self, args: t.Iterable[str], msg_regex: str = ""
) -> None:
with pytest.raises((CommandError, E2ETestSystemExit), match=msg_regex) as ex:
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL: https://github.com/pytest-dev/pytest/blob/main/src/_pytest/python_api.py#L783

self.cli_call(*args)
Expand Down
6 changes: 4 additions & 2 deletions python-threatexchange/threatexchange/exchanges/clients/fb_threatexchange/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,9 @@ def __init__(
def app_id(self):
return int(self.api_token.partition("|")[0])

def get_json_from_url(self, url, params=None, *, json_obj_hook: t.Callable = None):
def get_json_from_url(
self, url, params=None, *, json_obj_hook: t.Optional[t.Callable] = None
):
"""
Perform an HTTP GET request, and return the JSON response payload.
Same timeouts and retry strategy as `_get_session` above.
Expand Down Expand Up @@ -293,7 +295,7 @@ def get_threat_updates(
types: t.Iterable[str] = (),
page_size: t.Optional[int] = None,
fields: t.Optional[t.Iterable[str]] = None,
decode_fn: t.Callable[[t.Any], t.Any] = None,
decode_fn: t.Optional[t.Callable[[t.Any], t.Any]] = None,
) -> _CursoredResponse:
"""Gets threat updates for the given privacy group."""

Expand Down
2 changes: 1 addition & 1 deletion python-threatexchange/threatexchange/exchanges/signal_exchange_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ class SignalExchangeAPIWithSimpleUpdates(
state.TFetchCheckpoint,
state.TFetchedSignalMetadata,
t.Tuple[str, str],
state.TFetchedSignalMetadata,
t.Any,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Dcallies I'm not sure on the typing here. All changes besides t.Any expanded to 10 more errors where SignalExchangeAPIWithSimpleUpdates is being used.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, this is definitely not right here, the other option is to just # type: ignore for now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't get mypy to understand the # type: ignore directive here. No matter where I put it (above, same line, below as part of class) it get a version of the following:

threatexchange/exchanges/signal_exchange_api.py:269: error: Variance of TypeVar "state.TFetchedSignalMetadata" incompatible with variance in parent type  [type-var]
threatexchange/exchanges/signal_exchange_api.py:277: error: Unused "type: ignore" comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Synced offline, as ignore was not working I will merge as t.Any but created high-pri issue #1255

]
):
"""
Expand Down
2 changes: 1 addition & 1 deletion python-threatexchange/threatexchange/tests/hashing/test_pdq_faiss_matcher.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
]

MAX_UNSIGNED_INT64 = numpy.iinfo(numpy.uint64).max
MAX_UNSIGNED_INT64 = numpy.iinfo(numpy.int64).max


class MixinTests:
Expand Down