Skip to content

Conversation

@Borda
Copy link
Collaborator

@Borda Borda commented Aug 4, 2025

Before submitting
  • Was this discussed/agreed via a Github issue? (no need for typos and docs improvements)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure to update the docs?
  • Did all existing and newly added tests pass locally?

What does this PR do?

Addressing #371 (comment)

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in GitHub issues there's a high chance it will not be merged.


📚 Documentation preview 📚: https://lit-utilities--419.org.readthedocs.build/en/419/

@github-actions github-actions bot added ci/cd Continuous integration and delivery tests labels Aug 4, 2025
@Borda Borda requested a review from Copilot August 4, 2025 15:41
Copy link
Contributor

Copilot AI left a comment

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 fixes argument parsing behavior in CLI scripts by updating jsonargparse usage. The changes standardize how optional arguments are handled across multiple scripts, ensuring they are parsed as optional parameters rather than positional ones.

Key changes:

  • Updates auto_cli calls to use as_positional=False parameter
  • Adds comprehensive test coverage for different argument parsing scenarios
  • Affects three script files that use jsonargparse for CLI parsing

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
tests/scripts/test_adjust_torch_versions.py Adds parametrized test to verify positional, optional, and mixed argument parsing
scripts/inject-selector-script.py Updates auto_cli call to disable positional argument parsing
scripts/adjust-torch-versions.py Updates auto_cli call to disable positional argument parsing
.github/scripts/find-unused-caches.py Updates auto_cli call to disable positional argument parsing
Comments suppressed due to low confidence (1)

tests/scripts/test_adjust_torch_versions.py:41

  • [nitpick] The variable name main_params is ambiguous. Consider renaming it to script_parameters or cli_parameters to better indicate its purpose.
    main_params = (("requirements_path", path_req_file), ("torch_version", "1.10.0"))

@Borda Borda merged commit 757ea65 into main Aug 4, 2025
59 checks passed
@Borda Borda deleted the cli/scripts branch August 4, 2025 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/cd Continuous integration and delivery tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant