major refactoring #59
Conversation
WalkthroughThe changes include a new ignore rule in .gitignore and significant updates to the README.md, detailing revised installation instructions, expanded usage examples, and additional CLI commands. Several classes across the pridepy package have been reformatted for consistency, with new methods added for category‐based file downloads and updated command names in the CLI. Test files have received both new tests and formatting improvements, while API handling methods have been consolidated for readability, without altering overall functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as CLI
participant F as Files Module
participant A as API Server
U->>C: Execute download-all-public-category-files command
C->>F: Call download_all_category_files(accession, category, options)
F->>A: Request file list for specified category
A-->>F: Return file list
F-->>C: Send files data
C-->>U: Display download results
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
pridepy/pridepy.py (1)
55-67: Inconsistent default bandwidth value in docstring vs parameter.
The docstring states that the default is “100M,” while the parameter is set to “50M.” This discrepancy can cause confusion.You can either change the parameter’s default to match the docstring or adjust the docstring to match the code, for example:
- aspera_maximum_bandwidth: str = "50M", + aspera_maximum_bandwidth: str = "100M",
🧹 Nitpick comments (5)
pridepy/tests/test_raw_files.py (1)
32-39: Expand test coverage for additional file categories.
Currently, only “RAW” and “SEARCH” file categories are tested. Additional tests for other categories or error scenarios (e.g., invalid categories) can improve confidence in functionality.pridepy/authentication/authentication.py (1)
54-54: Simplified return statement.
Returning a concise Boolean expression is clear. If more debugging details are ever required, consider logging token validation failures.README.md (3)
26-27: Consider adding output examples.The installation commands would be more helpful with example output to show users what to expect.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
26-26: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
27-27: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
114-115: Add missing comma.Add a comma after "project" in the sentence: "...files from a project, which is the most common use case."
🧰 Tools
🪛 LanguageTool
[uncategorized] ~115-~115: Possible missing comma found.
Context: ...ct command to download RAW files from a project which is the most common use case. ## ...(AI_HYDRA_LEO_MISSING_COMMA)
136-136: Improve wording.Replace "At the moment" with "Currently" for more concise wording.
🧰 Tools
🪛 LanguageTool
[style] ~136-~136: For conciseness, consider replacing this expression with an adverb.
Context: ...e https protocol to download the files. At the moment we only allow this protocol because of ...(AT_THE_MOMENT)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.gitignore(1 hunks)README.md(2 hunks)pridepy/authentication/authentication.py(2 hunks)pridepy/files/files.py(13 hunks)pridepy/pridepy.py(7 hunks)pridepy/project/project.py(3 hunks)pridepy/tests/test_raw_files.py(1 hunks)pridepy/tests/test_search.py(1 hunks)pridepy/util/api_handling.py(4 hunks)
✅ Files skipped from review due to trivial changes (4)
- .gitignore
- pridepy/tests/test_search.py
- pridepy/project/project.py
- pridepy/util/api_handling.py
🧰 Additional context used
📓 Learnings (1)
pridepy/files/files.py (1)
Learnt from: ypriverol
PR: PRIDE-Archive/pridepy#53
File: pridepy/project/project.py:69-79
Timestamp: 2025-02-06T18:56:21.132Z
Learning: The `get_files_by_accession` method in `pridepy/project/project.py` no longer supports filtering in API v3, as it returns all files for a project using the stream endpoint.
🪛 Ruff (0.8.2)
pridepy/files/files.py
367-367: Function definition does not bind loop variable progress
(B023)
🪛 markdownlint-cli2 (0.17.2)
README.md
26-26: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
27-27: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
71-71: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
91-91: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
109-109: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
124-124: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
132-132: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
145-145: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
151-151: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
156-156: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
🪛 LanguageTool
README.md
[style] ~64-~64: Consider using a different verb to strengthen your wording.
Context: ...metadata... ``` > [!NOTE] > Please make sure you are using Python3, not Python 2.7 v...
(MAKE_SURE_ENSURE)
[style] ~75-~75: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...o specify the output directory. - -p flag is used to specify the protocol (**aspe...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~115-~115: Possible missing comma found.
Context: ...ct command to download RAW files from a project which is the most common use case. ## ...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~136-~136: For conciseness, consider replacing this expression with an adverb.
Context: ...e https protocol to download the files. At the moment we only allow this protocol because of ...
(AT_THE_MOMENT)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (8)
pridepy/pridepy.py (1)
320-327: Missing click options for parameters referenced in the function.
Parameters “filter,” “page_size,” “page,” “date_gap,” “sort_direction,” and “sort_fields” are used inside the function but lack corresponding click options. This will cause errors or unexpected behavior when running the command.Would you like help adding these options to ensure the command is fully operational?
pridepy/authentication/authentication.py (1)
30-30: Concise JSON credential string.
Shifting to a single‐line JSON string improves readability without altering logic.pridepy/files/files.py (6)
73-73: LGTM!The formatting changes improve code readability while maintaining the same functionality.
Also applies to: 79-81
89-89: LGTM!The simplified function call improves code readability.
705-709: LGTM!The reformatted function call improves code readability.
714-716: LGTM!The addition of S3 protocol support enhances the download capabilities of the package.
718-748: LGTM!Well-structured new function that enables category-based file downloads while reusing existing download functionality.
750-762: LGTM!Well-implemented function that efficiently filters files by category.
Major refactoring:
Summary by CodeRabbit
New Features
Documentation