Fix boolean conversion in DeferredImportIndicator class

[eminyouskn]

Fixes # NA

Summary/Motivation:

Some linters, such as Pyright, raise warnings when an object of class DeferredImportIndicator is used in an if statement, since its __bool__ method may return None.
This PR explicitly casts the result of that method to bool to ensure a proper boolean value is returned.

Changes proposed in this PR:

• Cast the result of __bool__ method of DeferredImportIndicator to be a bool

Legal Acknowledgement

By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution:

1. I agree my contributions are submitted under the BSD license.
2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

[jsiirola]

This is a nonfunctional change in that _available will always be bool after calling resolve(). I am curious: If you implement this change instead, does the linter recognize that fact?

diff --git a/pyomo/common/dependencies.py b/pyomo/common/dependencies.py
index 27167f12f..017b76c2c 100644
--- a/pyomo/common/dependencies.py
+++ b/pyomo/common/dependencies.py
@@ -344,13 +344,13 @@ class DeferredImportIndicator(_DeferredImportIndicatorBase):
         if callback is not None:
             DeferredImportCallbackFinder._callbacks.setdefault(name, []).append(self)

-    def __bool__(self):
+    def __bool__(self) -> bool:
         self.resolve()
         return self._available

     def resolve(self):
         # Only attempt the import once, then cache some form of result
-        if self._module is None:
+        if self._available is None:
             package = self._original_globals.get('__name__', '')
             try:
                 self._module, self._available = _perform_import(
(

[eminyouskn]

Even with the proposed changes it does not managed to infer that _available has been edited to be a bool in the resolve function. If you don't like the bool casting, how about create another flag _import_attempted and from the begining put _available to False et not to None? I assume that the None status of _available is only to know if the import has been attempted or not.

[jsiirola]

OK - I have thought about this a lot. The lack of a cast was previously defensive programming so that if - for whatever reason - resolve() failed to set _available to a bool, then Python would raise an exception. I think I would still like that behavior, so I suggest we add an assertion and then a couple comments explaining why things look weird.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.32%. Comparing base (e8ed5dd) to head (e27bd21).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3746   +/-   ##
=======================================
  Coverage   89.31%   89.32%           
=======================================
  Files         896      896           
  Lines      103697   103698    +1     
=======================================
+ Hits        92619    92626    +7     
+ Misses      11078    11072    -6     

Flag	Coverage Δ
builders	29.09% <100.00%> (+<0.01%)	⬆️
default	85.93% <100.00%> (?)
expensive	35.86% <100.00%> (?)
linux	86.96% <100.00%> (-2.09%)	⬇️
linux_other	86.96% <100.00%> (+<0.01%)	⬆️
osx	83.10% <100.00%> (+<0.01%)	⬆️
win	85.23% <100.00%> (+<0.01%)	⬆️
win_other	85.23% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.