Update openbabel_helpers.py

[rolan701]

better aromatic handling

In general, when a local variable is assigned but never read, you should either remove the assignment (keeping the right-hand side call if it has side effects) or rename the variable to an "unused" naming convention if it is intentionally unused for documentation purposes. Here, sync_obmol_from_bodict(core3D) is called only for its side effects, and the reassignment to core3D is not used afterward. To fix the issue without changing behavior, we should keep the function call but drop the assignment to core3D.

Concretely, in molSimplify/Scripts/enhanced_structgen_functionality.py, locate the end of the function where core3D.bo_dict = bo is followed by core3D = sync_obmol_from_bodict(core3D). Replace that second line with a bare call sync_obmol_from_bodict(core3D). No additional imports, methods, or definitions are needed, and functionality remains the same because the object is still passed to sync_obmol_from_bodict and any in-place side effects occur as before; we just stop assigning its (unused) return value.

In general, to fix an unused local variable created by an assignment whose right-hand side has side effects, you should keep the call for its side effects but remove the unnecessary rebinding of the variable. That way, the behavior (side effects) is preserved while the dead assignment is eliminated.

Here, the best minimal fix is to call sync_obmol_from_bodict(core3D) as a standalone expression, without assigning its return value back to core3D. This preserves any in-place mutations that sync_obmol_from_bodict performs on the core3D object referenced by callers, while removing the unused local variable assignment. No imports, new methods, or additional definitions are required. Concretely, in molSimplify/Scripts/enhanced_structgen_functionality.py, around line 3431, replace:

core3D = sync_obmol_from_bodict(core3D)

with:

sync_obmol_from_bodict(core3D)

and leave the rest of the function unchanged.

In general, the fix is to avoid a bare except ...: pass and instead either (a) narrow the exception type and/or (b) handle it meaningfully, for example by logging it and continuing, or by documenting why it is intentionally ignored.

Here, the safest minimal-impact fix is:

• Keep the try/except so that a single bad bond does not break the aromaticity check.
• In the except block, add a brief explanatory comment and optionally a debug log that an error occurred when checking IsAromatic() for a bond. Since we must not assume a project-wide logger, we can use print with a clearly prefixed message, but to avoid changing runtime behavior too much, we can keep it very minimal and only emit the message; or, if we want zero output change, we can add just an explanatory comment (but that still leaves silent failures). The best compromise is a short, guarded log that explains the skip.
• Do not change the return type or logic of _count_aromatic in the non-error case.

All needed code changes are confined to molSimplify/utils/openbabel_helpers.py, within the _count_aromatic helper around lines 211–219. No new imports are strictly required; we will just add a comment and a simple debug print in the except block.

General fix: avoid a bare except that only does pass. Either (a) narrow the exception type and document why it is being ignored, or (b) log/record the exception so it’s not silent while still allowing the program to continue.

Best way here without changing functionality: keep catching Exception so the control flow is unchanged, but add a diagnostic message to stderr when ff.WeightedRotorSearch fails, ideally only when verbose_ff is enabled (to avoid noisy output in normal runs). This preserves the “non-fatal” nature of the rotor search while ensuring failures are visible when debugging. Since we can’t see the top of the file’s imports beyond what’s shown, we should avoid adding new imports and instead use print(..., file=sys.stderr) only if sys is already imported in this file; we have not been shown that. To stay safe and avoid new imports outside the shown region, we can use a simple print guarded by verbose_ff, which is already in use elsewhere in the function and presumably in scope.

Concretely:

• Replace the except Exception: pass block around ff.WeightedRotorSearch (lines 339–342) with an except Exception as e: block that:
  • Optionally prints a short message if verbose_ff is true, including the exception message.
  • Contains a brief comment indicating the rotor search is optional and failures are non-fatal.
    This change stays within the shown function and does not alter any other behavior.

In general, to fix an empty except block you should either narrow the exception type and handle it explicitly (e.g., fallback behavior, re-raise, cleanup) or at least log the error so that failures are visible instead of silently ignored.

For this specific block, the surrounding comment indicates the intent: some force fields may not implement GetVDWEnergy, and the code wants to “guard” this call and keep running. The best non-disruptive fix is therefore:

• Keep the try/except structure and preserve the existing semantics that vdw remains None if the call fails.
• Add minimal logging inside the except block to record that GetVDWEnergy failed and why, without changing return values.
• Use the standard library logging module (safe, well-known, no external dependency), adding an import at the top of the file.
• Optionally, catch a broad Exception but log the stack trace with logging.exception, which preserves diagnostics while still not raising.

Concretely:

• In molSimplify/utils/openbabel_helpers.py, add import logging alongside the other imports at the top.
• Replace the except Exception: pass block around ff.GetVDWEnergy() with an except Exception as e: block that calls logging.exception(...) with a brief message, e.g., "Failed to compute VDW energy via ff.GetVDWEnergy()".
• Do not change how vdw is set or how results is returned.

---

[codecov]

Codecov Report

❌ Patch coverage is 0% with 303 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.26%. Comparing base (de1a530) to head (a5a4152).
⚠️ Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
molSimplify/utils/openbabel_helpers.py	0.00%	229 Missing ⚠️
...mplify/Scripts/enhanced_structgen_functionality.py	0.00%	55 Missing ⚠️
molSimplify/Scripts/enhanced_structgen.py	0.00%	19 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #534      +/-   ##
==========================================
- Coverage   40.58%   40.26%   -0.32%     
==========================================
  Files          93       93              
  Lines       30079    30325     +246     
==========================================
+ Hits        12207    12211       +4     
- Misses      17872    18114     +242     

Flag	Coverage Δ
unittests	40.26% <0.00%> (-0.32%)	⬇️

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.