osqp / osqp-python

Python interface for OSQP
https://osqp.org/
Apache License 2.0
109 stars 41 forks source link

failing as we should, instead of printing out an error #157

Closed vineetbansal closed 2 months ago

vineetbansal commented 2 months ago

I'm not sure why the following print statements were put in - merely printing out an error and proceeding like nothing happened doesn't accomplish anything IMO, and only hides the otherwise invaluable stack trace. It's better to fail explicitly with an ImportError.

I noticed this when a bad installation of torch in my environment was preventing an internal import deep inside pytorch (triggered in our code on from torch.nn import Module), but this was hard to track down because of this print.

If the intention here was to allow osqp code execution even in the absence of the torch module, that's understandable, but again, no other code in osqp seems to be inadvertently picking up this module where it shouldn't except a unit test, and doing an from osqp.nn.torch import .. should trigger an ImportError.

CLAassistant commented 2 months ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Vineet Bansal seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.