Skip to content

Reapply AZ Python fixes#198

Open
shreyasvinaya wants to merge 7 commits into
mainfrom
reapply-az-py-fixes
Open

Reapply AZ Python fixes#198
shreyasvinaya wants to merge 7 commits into
mainfrom
reapply-az-py-fixes

Conversation

@shreyasvinaya
Copy link
Copy Markdown
Collaborator

@shreyasvinaya shreyasvinaya commented Apr 13, 2026

Reapplies the AZ changes from #196 after the revert in #197.

@shreyasvinaya shreyasvinaya changed the base branch from revert-az-py-fixes to main April 13, 2026 15:11
@shreyasvinaya shreyasvinaya mentioned this pull request Apr 20, 2026
10 tasks
Copy link
Copy Markdown
Collaborator

@riya-singh28 riya-singh28 left a comment

Choose a reason for hiding this comment

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

@shreyasvinaya I have put some comments after the first review.

Comment thread deepretro/tests/test_az.py Outdated
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@shreyasvinaya Please shift all the fixtures to conftest.py file.

Comment thread deepretro/utils/az.py Outdated
]


def _get_aizynthfinder_cls():
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@shreyasvinaya Is a function required here instead of just try-except at the beginning of the file?

Comment thread deepretro/utils/az.py


@cache_results
def run_az_with_img(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@shreyasvinaya Why is the following try-except block used in run_az, but not in run_az_with_img function?

try:

        config_path = f"{AZ_MODELS_PATH}/{az_model}/config.yml"

        with open(config_path, "r") as _:

            config_filename = config_path

    except FileNotFoundError:

        logger.warning("AZ config not found, trying fallback", path=config_path)

        try:

            with open(AZ_MODEL_CONFIG_PATH, "r") as _:

                config_filename = AZ_MODEL_CONFIG_PATH

        except FileNotFoundError:

            raise FileNotFoundError(

                f"AZ_MODEL_CONFIG_PATH not found at {AZ_MODEL_CONFIG_PATH}"

            )

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

handled with the common _resolve_config function

Comment thread deepretro/utils/az.py
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@shreyasvinaya I compared run_az function with the run_az_with_img, and mostly it looks the same except for the image computation and output sizes. In general, this may cause issues in the future if someone updates, given that any change made in both functions.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Update: since the core for both are the same, there is now a private function that does the common parts, each of the individual functions now format the output of the private function in their respective formats

shreyasvinaya added a commit that referenced this pull request May 22, 2026
- Replace _get_aizynthfinder_cls() with top-level try-except import
- Extract _resolve_config() and _run_finder() helpers shared by both
  run_az and run_az_with_img, so config fallback logic is consistent
- Move test fixtures from test_az.py to conftest.py for reuse

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
shreyasvinaya and others added 3 commits May 22, 2026 21:39
- Replace _get_aizynthfinder_cls() with top-level try-except import
- Extract _resolve_config() and _run_finder() helpers shared by both
  run_az and run_az_with_img, so config fallback logic is consistent
- Move test fixtures from test_az.py to conftest.py for reuse

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extract basic-molecule check, config resolution, finder setup, and
result extraction into _run_az_core so the two public functions are
thin wrappers handling only caching and return-tuple shape. Add tests
for _run_az_core directly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…_MOLECULES)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add actions/cache step to persist downloaded models between runs
- Download models in a dedicated step (only on cache miss)
- Fixture honours AZ_TEST_MODELS_DIR env var to skip re-download
- _download_aizynth_models skips if config.yml already present

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.

2 participants