improve skip options

[ypriverol]

PR Type

Enhancement

---

Description

• Convert skip_if_downloaded_already option to Click boolean flag

• Standardize flag naming with hyphens across all commands

• Improve help text clarity and consistency

• Update documentation with new flag usage examples

---

Diagram Walkthrough

flowchart LR
  A["Old: -skip flag<br/>String-based option"] -- "Refactor" --> B["New: --skip-if-downloaded-already<br/>Boolean flag"]
  B -- "Applied to" --> C["4 CLI commands"]
  D["Help text<br/>inconsistencies"] -- "Standardize" --> E["Consistent help<br/>across commands"]
  F["README<br/>documentation"] -- "Update" --> G["New flag syntax<br/>and examples"]

Loading

File Walkthrough

	Relevant files
Enhancement	pridepy.py Convert skip option to boolean flag format                              pridepy/pridepy.py Converted skip_if_downloaded_already from string option to boolean flag in 4 CLI commands Changed flag syntax from -skip to --skip-if-downloaded-already/--no-skip-if-downloaded-already Updated help text to be consistent: "Skip the download if the file has already been downloaded. Default is to skip." Applied changes to main(), download_all_public_raw_files(), download_all_public_category_files(), and download_px_raw_files() commands +12/-16
Documentation	README.md Update documentation for new skip flag syntax                        README.md Updated documentation to reflect new flag naming convention Added explanation of both --skip-if-downloaded-already and --no-skip-if-downloaded-already options Clarified default behavior and how to force re-download +1/-1

---

Summary by CodeRabbit

• Documentation

  • Clarified download behavior and updated docs to recommend using --skip-if-downloaded-already to avoid re-downloading; notes now state resume is supported.

• Updates

  • CLI option names normalized to kebab-case for consistency (e.g., output-folder, file-name, page-size, sort-direction, checksum-check, aspera-maximum-bandwidth).
  • Skip behavior changed: by default files are re-downloaded; use --skip-if-downloaded-already to skip existing files.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Normalized CLI option names to kebab-case and replaced the previous skip flag with a flag-style --skip-if-downloaded-already (default False). README wording updated to reflect resume support and the new skip flag behavior.

Changes

Cohort / File(s)	Summary
Documentation README.md	Updated skip/resume wording: removed note that skip is default; now documents "Resume is supported. Use --skip-if-downloaded-already to skip files already downloaded."
CLI flags & commands pridepy/pridepy.py	Renamed many CLI options from snake_case to kebab-case (examples: output_folder → output-folder, file_name → file-name, page_size → page-size, sort_direction → sort-direction, sort_fields → sort-fields, aspera_maximum_bandwidth → aspera-maximum-bandwidth, checksum_check → checksum-check). Changed skip behavior: skip_if_downloaded_already → --skip-if-downloaded-already implemented as is_flag=True, default=False, affecting download-all-public-raw-files, download-all-public-category-files, download-file-by-name, download-px-raw-files. Also renamed output flags for metadata commands (output_file → output-file). Help text and decorators updated accordingly.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Focus areas:
  • pridepy/pridepy.py — verify each CLI decorator/name change, default values, and help text consistency.
  • Ensure backward-compatibility expectations and CLI parsing (aliases/short flags) are intentional.
  • README.md — verify wording accurately reflects runtime behavior.

Possibly related PRs

• move to poetry #61 — modifies download/stream commands and the skip/download handling referenced here.
• Search by key word #60 — touches CLI options for search and download commands (page_size/sort_fields) that overlap.
• ProteomeXchange support #67 — updates ProteomeXchange-related download flags, including skip/download behavior for PX commands.

Suggested labels

Review effort 3/5

Suggested reviewers

• selvaebi

Poem

I hop through flags with a joyful bound,
Turning underscores into kebab-shaped sound.
Skip now a switch you can freely set,
Downloads resume — no fret, no sweat. 🐰

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'improve skip options' accurately captures the main objective of the PR—converting skip functionality to a boolean flag with standardized naming conventions.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch dev

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 74d8b47 and 8842e9c.

📒 Files selected for processing (2)

• README.md (2 hunks)
• pridepy/pridepy.py (11 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: build
• GitHub Check: build

🔇 Additional comments (7)

README.md (1)

83-96: LGTM!

The documentation accurately reflects the new flag naming (kebab-case) and default behavior. The explanations are clear and correctly describe that files are re-downloaded by default unless --skip-if-downloaded-already is used.

pridepy/pridepy.py (6)

27-49: LGTM!

The CLI options have been successfully standardized to kebab-case naming, and the is_flag=True configuration is appropriate for the single-flag format requested by the maintainer.

---

100-122: LGTM!

CLI options consistently use kebab-case naming.

---

181-208: LGTM!

All CLI options consistently use kebab-case naming, including the newly added file-name option.

---

253-281: LGTM!

The CLI options are properly configured with kebab-case naming and the skip flag correctly defaults to False.

---

304-339: LGTM!

The output-file option naming is consistent with the kebab-case standardization across the codebase.

---

342-413: LGTM!

All search-related options (page-size, sort-direction, sort-fields) consistently use kebab-case naming.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: No audit logs: The new CLI flag handling adds no logging for critical actions (e.g., when downloads are skipped vs performed), making it unclear if audit trails capture user actions and outcomes. Referred Code @click.option( "--skip-if-downloaded-already/--no-skip-if-downloaded-already", is_flag=True, default=True, help="Skip the download if the file has already been downloaded. Default is to skip.", ) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Missing validation: The added flag options introduce execution branches (skip vs no-skip) without visible validation or error handling for edge cases in this diff segment. Referred Code @click.option( "--skip-if-downloaded-already/--no-skip-if-downloaded-already", is_flag=True, default=True, help="Skip the download if the file has already been downloaded. Default is to skip.", ) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Input defaults: The new boolean flag defaults to True which affects download behavior, but the diff shows no accompanying validation or authorization checks related to file operations. Referred Code @click.option( "--skip-if-downloaded-already/--no-skip-if-downloaded-already", is_flag=True, default=True, help="Skip the download if the file has already been downloaded. Default is to skip.", ) Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Consolidate repeated CLI options Refactor the duplicated click.option decorators across multiple commands into a single, reusable decorator function. This will reduce code repetition and improve maintainability. Examples: pridepy/pridepy.py [18-49] @click.option("-a", "--accession", required=True, help="PRIDE project accession") @click.option( "-p", "--protocol", default="ftp", help="Protocol to be used to download files either by ftp or aspera or from globus. Default is ftp", ) @click.option( "-o", "--output_folder", ... (clipped 22 lines) pridepy/pridepy.py [91-122] @click.option("-a", "--accession", required=True, help="PRIDE project accession") @click.option( "-p", "--protocol", default="ftp", help="Protocol to be used to download files either by ftp or aspera or from globus. Default is ftp", ) @click.option( "-o", "--output_folder", ... (clipped 22 lines) Solution Walkthrough: Before: @main.command(...) @click.option("-a", "--accession", ...) @click.option("-p", "--protocol", ...) @click.option("-o", "--output_folder", ...) @click.option("--skip-if-downloaded-already/--no-skip-if-downloaded-already", ...) @click.option("--aspera_maximum_bandwidth", ...) def download_all_public_raw_files(...): # ... command logic @main.command(...) @click.option("-a", "--accession", ...) @click.option("-p", "--protocol", ...) @click.option("-o", "--output_folder", ...) @click.option("--skip-if-downloaded-already/--no-skip-if-downloaded-already", ...) @click.option("--aspera_maximum_bandwidth", ...) def download_all_public_category_files(...): # ... command logic After: def common_download_options(f): options = [ click.option("-a", "--accession", ...), click.option("-p", "--protocol", ...), click.option("-o", "--output_folder", ...), click.option("--skip-if-downloaded-already/--no-skip-if-downloaded-already", ...), click.option("--aspera_maximum_bandwidth", ...) ] for option in reversed(options): f = option(f) return f @main.command(...) @common_download_options def download_all_public_raw_files(...): # ... command logic @main.command(...) @common_download_options def download_all_public_category_files(...): # ... command logic Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies significant code duplication of click.option decorators and proposes a valid refactoring pattern that would improve long-term maintainability.	Medium
General	Add a short alias for flag Add a short alias, such as -s, to the --skip-if-downloaded-already flag to improve CLI usability. pridepy/pridepy.py [31-36] @click.option( + "-s", "--skip-if-downloaded-already/--no-skip-if-downloaded-already", + "skip_if_downloaded_already", is_flag=True, default=True, help="Skip the download if the file has already been downloaded. Default is to skip.", ) Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion correctly proposes adding a standard short alias (-s) for a long command-line flag, which improves the tool's usability and follows common CLI best practices.	Low
Update

[Copilot]

Pull request overview

This PR converts the skip_if_downloaded_already option from a string-based parameter to a proper boolean flag across four CLI download commands, improving the Click interface consistency and user experience.

Key Changes:

• Converted skip_if_downloaded_already to a boolean flag with --skip-if-downloaded-already/--no-skip-if-downloaded-already syntax
• Standardized help text across all four affected commands
• Updated README documentation to reflect new flag usage

Reviewed changes

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

File	Description
pridepy/pridepy.py	Converts skip option to boolean flag format in 4 CLI commands (download-all-public-raw-files, download-all-public-category-files, download-file-by-name, download-px-raw-files)
README.md	Updates documentation to describe the new flag syntax and behavior

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The new flag uses hyphens (--skip-if-downloaded-already), but other flags in the codebase use underscores (--output_folder, --file_name, --aspera_maximum_bandwidth, --checksum_check). This creates inconsistency in the CLI interface. Consider either keeping underscores to match existing conventions or updating all flags to use hyphens for consistency.

[ypriverol]

@copilot lets move everything to hyphens for consistency.

[Copilot]

@ypriverol I've opened a new pull request, #75, to work on those changes. Once the pull request is ready, I'll request review from you.

[Copilot]

Pull request overview

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

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

When using the dual flag format (--flag/--no-flag) in Click, the is_flag=True parameter is redundant and unnecessary. The dual flag format automatically creates a boolean option. Consider removing is_flag=True to follow Click's best practices.

[Copilot]

@ypriverol I've opened a new pull request, #76, to work on those changes. Once the pull request is ready, I'll request review from you.