PyRIT icon indicating copy to clipboard operation
PyRIT copied to clipboard

Warnings in pipelines

Open romanlutz opened this issue 1 year ago • 2 comments

Describe the bug

The following shows up in our pipelines (and repros locally with pre-commit run --all-files)

pyrit/prompt_target/azure_blob_storage_target.py:77: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
pyrit/prompt_converter/translation_converter.py:114: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
doc/code/orchestrators/xpia_helpers.py:180: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]

Steps/Code to Reproduce

pre-commit run --all-files

Expected Results

no warnings or errors

Actual Results

see above

Note that we should try to fix the issue (i.e., make it a typed function) rather than ignoring the issue.

romanlutz avatar Oct 09 '24 16:10 romanlutz

Hi, @romanlutz. I opened a PR to add annotations for the return types of those methods.

However, when running pre-commit run --all-files locally, I was not able to reproduce those warnings. Would it be possible to link to an Actions run that had them? I looked through some recent runs but was not able to find them.

Relatedly, I noticed that there are other methods, such as _create_container_client_async in xpia_helpers.py and send_prompt_async in fuzzer_converter_base.py, without annotations for return types. Were there warnings for these methods as well by chance?

Tiger-Du avatar Oct 18 '24 01:10 Tiger-Du

Thanks @Tiger-Du!

As is, mypy will only print those warnings if there is also an error. And you're correct that the additional methods are new warnings.

I approved your PR, but IMO we should configure mypy to eventually have errors (in pyproject.toml making strict true). However, that will be too big of a change for one PR. But one thing we can do is

disallow_untyped_defs = true

And we could tackle these all at once. That's my pref for the existing PR - we could probably create a new issue to gradually make mypy strict :)

rlundeen2 avatar Oct 18 '24 03:10 rlundeen2