Azure / PyRIT

The Python Risk Identification Tool for generative AI (PyRIT) is an open access automation framework to empower security professionals and machine learning engineers to proactively find risks in their generative AI systems.
MIT License
1.91k stars 366 forks source link

Warnings in pipelines #442

Open romanlutz opened 1 month ago

romanlutz commented 1 month ago

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.

Tiger-Du commented 1 month ago

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?

rlundeen2 commented 1 month ago

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 :)