Skip to content

Prevent MP2 localizer deprecation warnings during import - #691

Open
David Williams-Young (wavefunction91) with Copilot wants to merge 5 commits into
mainfrom
copilot/fix-issue-572-deprecation-warning
Open

Prevent MP2 localizer deprecation warnings during import#691
David Williams-Young (wavefunction91) with Copilot wants to merge 5 commits into
mainfrom
copilot/fix-issue-572-deprecation-warning

Conversation

Copilot AI commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Import-time registry stub generation constructed the deprecated MP2 natural-orbital localizer through the public factory path, emitting Python and C++ warnings.

  • Factory lifecycle

    • Added a private introspection construction path without user-facing warnings.
    • Preserved warnings for explicit qdk_mp2_natural_orbitals creation.
  • Stub generation

    • Reused one unwrapped instance for type and settings inspection.
  • Regression coverage

    • Verifies package import is warning-free.
    • Verifies explicit creation emits one Python warning and one C++ log message.
Original prompt

Reproduce and fix issue #572 on current main in a clean cloud environment.

Importing qdk_chemistry must not emit the MP2NaturalOrbitalLocalizer deprecation message through either Python warnings or the C++ logger. Explicitly requesting the deprecated qdk_mp2_natural_orbitals implementation must continue to emit the intended deprecation warning exactly at the user-facing creation boundary.

Find and correct the underlying registration or construction lifecycle that causes an internal import-time operation to be treated as user-requested deprecated usage. Do not solve this by suppressing, filtering, catching, redirecting, or globally disabling warnings or logging, and do not add an environment-variable escape hatch. Preserve normal deprecation behavior for explicit use.

Implement the minimum focused fix and add regression tests proving both warning-free package import and retained explicit deprecation behavior. Avoid unrelated deprecation or registry refactoring. Follow the repository instructions and skills, run the smallest relevant Python tests and configured lint checks, and use "Fixes #572" in the pull request description.

Co-authored-by: wavefunction91 <6208701+wavefunction91@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix issue #572 to prevent MP2NaturalOrbitalLocalizer deprecation message Prevent MP2 localizer deprecation warnings during import Aug 28, 2026
@wavefunction91

Copy link
Copy Markdown
Collaborator

Copilot Pick this PR back up and finish it. Do not rebuild or reinstall qdk-chemistry, and do not run the full test suite, documentation build, or broad pre-commit set on the agent runner. Use the focused regression test already added in this PR as the local validation: python -m pytest -q python/tests/test_mp2_natural_orbital_deprecation.py. If the existing environment cannot run that test without rebuilding, do not substitute a package build; inspect the implementation and rely on the PR CI jobs for compiled validation. Confirm by source inspection that import-time registry/stub introspection uses the non-warning construction path, while explicit public factory creation still emits exactly the intended Python deprecation warning and C++ log warning. Review the diff for a minimum, principled lifecycle fix rather than warning suppression. The current workflow has a macOS failure, but its logs are unavailable until the still-running workflow completes; inspect and address it only if it is caused by this PR. Keep the PR focused on #572.

Co-authored-by: wavefunction91 <6208701+wavefunction91@users.noreply.github.com>

Copilot AI commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Copilot Pick this PR back up and finish it. Do not rebuild or reinstall qdk-chemistry, and do not run the full test suite, documentation build, or broad pre-commit set on the agent runner. Use the fo...

Finished in 613e4a9. Source inspection confirms stub introspection uses the non-warning factory path, while public creation retains one Python warning and one C++ log warning. The requested local test could not run because pytest is absent, so I did not rebuild or install. The macOS failure was caused by the new test inheriting a disabled global logger; the test now establishes and restores warning-level logging.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: b7eef1a1-8f39-42f8-8893-261a5f5a7f35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR prevents MP2 natural-orbital localizer deprecation warnings from being emitted during qdk_chemistry import-time registry stub generation by introducing an internal “introspection” construction path that bypasses deprecation emission, while preserving existing deprecation behavior for explicit user creation via the public factory APIs.

Changes:

  • Added an internal factory creation path (FactoryAccess / _create_for_introspection) that constructs instances without triggering deprecation warnings/logs.
  • Updated registry stub generation to reuse a single introspected instance for both type and settings inspection.
  • Added regression tests covering warning-free stub generation and preserved explicit deprecation behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
python/tests/test_mp2_natural_orbital_deprecation.py Adds regression tests for warning-free stub generation and one-time deprecation on explicit creation.
python/src/qdk_chemistry/algorithms/registry.py Refactors settings inspection into a helper and adds internal introspection instance creation.
python/src/qdk_chemistry/init.py Switches stub generation to use internal introspection construction + reused instance settings inspection.
python/src/pybind11/algorithms/factory_bindings.hpp Exposes an internal _create_for_introspection binding that bypasses deprecation emission.
cpp/include/qdk/chemistry/algorithms/algorithm.hpp Moves deprecation logging into public create() and adds FactoryAccess to call internal create_impl() without logging.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c61ff7f1-1ce7-4701-859b-0508efa9e4b9

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Comment on lines 258 to +263
* @param name The name to identify the desired algorithm implementation.
* @return A unique pointer to the created algorithm instance.
* @throws std::runtime_error if the name is not found in the registry.
*/
static return_type create(const std::string& name = "") {
static return_type create(const std::string& name = "",
bool suppress_warnings = false) {
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.

Deprecation warning after import

3 participants