search

stanfordnlp / dspy

DSPy: The framework for programming—not prompting—foundation models
https://dspy-docs.vercel.app/
MIT License
17.53k stars 1.34k forks source link

Incorrect usage of python assertions #1134

Open Bain-OS opened 3 months ago

Bain-OS commented 3 months ago

The way python assert statements are used here is potentially not correct. These should be raising exceptions instead

See these for example, I have not done an exhaustive search https://github.com/stanfordnlp/dspy/blob/main/dsp/templates/template_v2.py#L102 https://github.com/stanfordnlp/dspy/blob/main/dsp/templates/template_v2.py#L188

The reason this usage is incorrect: when deploying production python codebase it is good to set the optimize flag which removes asserts. i.e asserts should only be used as a development/debugging tool

see more here https://docs.datadoghq.com/code_analysis/static_analysis_rules/python-best-practices/no-assert/

arnavsinghvi11 commented 3 months ago

Hi @Bain-OS , thanks for raising this. These asserts were set up for the open-source development and testing of DSPy. Definitely agree that these should be removed in production environments, and it would be great if you have any ideas on keeping a flag for this within the codebase to make DSPy production ready. Feel free to open a PR on it!

tom-doerr commented 3 months ago

I think @Bain-OS is worried about the exact opposite. When you run a Python program in production you can set the optimize flag (-O) which results, among other things, in asserts being removed. This would mean no errors are thrown for the cases the asserts are checking for in production. I however can't tell if that is unwanted behavior or actually wanted in this case.