Skip to content

PR: better py2opsin handling - #23

Merged
r-fedorov merged 6 commits into
mainfrom
tests
Jul 1, 2026
Merged

PR: better py2opsin handling #23
r-fedorov merged 6 commits into
mainfrom
tests

Conversation

@r-fedorov

@r-fedorov r-fedorov commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Summary by Sourcery

Improve OPSIN verification handling and documentation while updating project metadata.

New Features:

  • Document default OPSIN behavior, verification statuses, and CLI vs Python API differences for name verification.

Bug Fixes:

  • Ensure py2opsin Java-related RuntimeWarnings are suppressed so OPSIN verification can fail gracefully when Java is unavailable.

Enhancements:

  • Clarify CLI verification flags and outputs when OPSIN support is unavailable.
  • Remove obsolete OPSIN QM9 batch test file.

Build:

  • Bump project version to 0.1.1 and raise minimum supported Python version to 3.11.
  • Update project URLs to point to the openclatura GitHub repository.

Tests:

  • Add a regression test to ensure py2opsin Java import warnings are suppressed and reported as skipped_no_java.

Summary by CodeRabbit

  • Documentation

    • Clarified OPSIN verification setup, including optional installation requirements, Java dependency, and how verification behaves when Java isn’t available.
    • Updated API and CLI usage notes to explain default verification behavior and how to disable it.
  • Bug Fixes

    • Suppressed an unnecessary warning when OPSIN verification can’t run because Java is unavailable.
  • Tests

    • Added coverage for the “verification skipped” case when Java is missing.
  • Chores

    • Updated package version and project links.

@sourcery-ai

sourcery-ai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Reviewer's Guide

Improves OPSIN/py2opsin handling by suppressing noisy Java warnings, clarifying verification behavior in docs and CLI, tightening Python version and project metadata, and adding tests to ensure OPSIN verification degrades gracefully when Java is unavailable.

Sequence diagram for OPSIN verification with graceful py2opsin/Java handling

sequenceDiagram
    actor User
    participant PythonAPI as openclatura_name
    participant OpsinVerifier as verify_opsin
    participant Py2opsinImporter as _try_import_py2opsin
    participant Py2opsin

    User->>PythonAPI: openclatura.name(verify_opsin=True)
    PythonAPI->>OpsinVerifier: verify_opsin(smiles, name)
    OpsinVerifier->>Py2opsinImporter: _try_import_py2opsin()
    Py2opsinImporter->>Py2opsin: import py2opsin (Java warnings ignored)
    Py2opsin-->Py2opsinImporter: py2opsin or None

    alt py2opsin is None
        OpsinVerifier-->>PythonAPI: opsin_check.status = skipped_no_opsin
    else Java unavailable
        OpsinVerifier-->>PythonAPI: opsin_check.status = skipped_no_java
    else OPSIN parses and round-trips
        OpsinVerifier-->>PythonAPI: opsin_check.status = matched/mismatched
    end

    PythonAPI-->>User: result with opsin_check.status and verified flag
Loading

File-Level Changes

Change Details Files
Suppress noisy RuntimeWarnings from py2opsin about missing/inaccessible Java while preserving graceful fallback behavior when Java is unavailable.
  • Wrap py2opsin import in a warnings context that filters RuntimeWarnings matching the 'Java may not be installed/accessible' message.
  • Ensure that failure to import py2opsin still returns None so OPSIN verification is treated as unavailable rather than raising.
src/openclatura/opsin_verify.py
Add tests to validate py2opsin Java warning suppression and the existing graceful skip behavior without Java.
  • Introduce imports for types and warnings in the public API test module to support new tests.
  • Add a test that monkeypatches import to emit a RuntimeWarning from py2opsin and verifies it is suppressed while the status becomes 'skipped_no_java'.
  • Reuse the existing opsin_verify module to confirm behavior when Java is unavailable.
src/openclatura/tests/test_public_api.py
Clarify documentation for OPSIN verification behavior in both the Python API and CLI, including default settings and skip statuses.
  • Explain that the default installation does not include OPSIN verification and show how to install the opsin extra and verify Java availability.
  • Document that verify_opsin=False by default in the Python API and list the possible opsin_check.status values for best-effort verification.
  • Clarify that the CLI verifies with OPSIN by default when possible, how to disable it, and what output/skip statuses to expect when OPSIN support is unavailable.
README.md
Adjust project metadata to reflect new release and repo location and tighten supported Python versions.
  • Bump project version from 0.1.0 to 0.1.1.
  • Increase minimum supported Python version from 3.10 to 3.11.
  • Update project URLs (Homepage, Repository, Issues) to point to the openclatura repo instead of the old iupac-name-generator repo.
pyproject.toml
Refine CLI help text for OPSIN verification flags and remove obsolete OPSIN batch test.
  • Clarify the --verify/--no-verify help text for both 'name' and 'batch' subcommands to emphasize round-trip via OPSIN and how to disable it.
  • Delete the obsolete test_opsin_qm9_batch.py file, presumably superseded by other tests or no longer needed.
src/openclatura/cli.py
test_opsin_qm9_batch.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@r-fedorov r-fedorov self-assigned this Jul 1, 2026
@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@r-fedorov, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 52 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Free

Run ID: 149686dc-0078-4e8a-bfc7-bd6f134dc87d

📥 Commits

Reviewing files that changed from the base of the PR and between c097348 and de3cdbb.

📒 Files selected for processing (2)
  • src/openclatura/opsin_verify.py
  • src/openclatura/tests/test_public_api.py
📝 Walkthrough

Walkthrough

Suppresses a Java-related RuntimeWarning during optional py2opsin import in opsin_verify.py, adds a corresponding test, updates CLI help text wording, bumps package version and Python requirement, updates project URLs, revises README documentation on OPSIN verification behavior, and removes a standalone QM9 batch test script.

Changes

OPSIN warning suppression, docs, and metadata

Layer / File(s) Summary
Suppress Java RuntimeWarning during py2opsin import
src/openclatura/opsin_verify.py
Wraps the py2opsin import in warnings.catch_warnings() with a filterwarnings rule ignoring a specific Java-unavailable RuntimeWarning.
Test coverage for suppressed warning
src/openclatura/tests/test_public_api.py
Adds a test that stubs the import to emit the Java warning and asserts verify_with_opsin returns skipped_no_java without surfacing the warning.
CLI help text wording update
src/openclatura/cli.py
Updates --verify help strings for name and batch subcommands to reference --no-verify.
README documentation and package metadata updates
README.md, pyproject.toml
Documents OPSIN verification defaults/skip statuses for install, Python API, and CLI; bumps version to 0.1.1, raises requires-python to >=3.11, and updates project URLs to openclatura.
Removed standalone QM9 batch test script
test_opsin_qm9_batch.py
Deletes the script and its canon, try_name_smiles, and main functions used for QM9 SMILES-to-name-to-SMILES round-trip accuracy testing.

Estimated code review effort: 2 (Simple) | ~10 minutes

Related PRs: None identified.

Suggested labels: documentation, dependencies

Suggested reviewers: None identified.


Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login.

Comment @coderabbitai help to get the list of available commands.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • The warning suppression in _try_import_py2opsin relies on a very specific message string from py2opsin; consider matching more robustly (e.g., on category or a broader substring) to avoid leaking noise if upstream tweaks the wording.
  • In test_py2opsin_java_import_warning_is_suppressed, the monkeypatch of builtins.__import__ affects all imports during the test; if more imports are added later, consider scoping this more tightly (e.g., patching only openclatura.opsin_verify.py2opsin) to reduce the chance of side effects.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The warning suppression in `_try_import_py2opsin` relies on a very specific message string from `py2opsin`; consider matching more robustly (e.g., on category or a broader substring) to avoid leaking noise if upstream tweaks the wording.
- In `test_py2opsin_java_import_warning_is_suppressed`, the monkeypatch of `builtins.__import__` affects all imports during the test; if more imports are added later, consider scoping this more tightly (e.g., patching only `openclatura.opsin_verify.py2opsin`) to reduce the chance of side effects.

Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@r-fedorov
r-fedorov merged commit 0f8add0 into main Jul 1, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant