🧹 Refactor find_path into backend-specific helpers#227
Conversation
The `find_path` function in `src/scida/convenience.py` was overly complex, handling local paths, remote URLs, and custom resources in a single block. This change: - Extracts HTTP/HTTPS logic into `_find_path_http`. - Extracts custom resource logic into `_find_path_resource`. - Extracts local datafolder search logic into `_find_path_local`. - Simplifies `find_path` to delegate to these helpers. - Ensures consistent return type (str) for `find_path`. This improves maintainability and readability without changing the external behavior. Co-authored-by: cbyrohl <9221545+cbyrohl@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Pull request overview
Refactors scida.convenience.find_path by splitting backend-specific path resolution logic into dedicated private helper functions, aiming to simplify the main dispatch flow and improve maintainability.
Changes:
- Extracted HTTP/HTTPS download/extract logic into
_find_path_http. - Extracted custom resource lookup into
_find_path_resource. - Extracted local
datafolderslookup into_find_path_localand standardizedfind_pathto always returnstr.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| print( | ||
| "Have not specified 'download_path' in config. Using 'cache_path' instead." | ||
| ) | ||
| savepath = config.get("cache_path") |
| # count folders in savepath | ||
| nfolders = len([f for f in extractpath.glob("*") if f.is_dir()]) | ||
| nobjects = len([f for f in extractpath.glob("*") if f.is_dir()]) | ||
| if nobjects == 1 and nfolders == 1: | ||
| extractpath = extractpath / [f for f in extractpath.glob("*") if f.is_dir()][0] |
| elif len(path.split(":")) > 1: | ||
| # check different alternative backends | ||
| databackend = path.split("://")[0] | ||
| dataname = path.split("://")[1] | ||
| if databackend in ["http", "https"]: |
Refactor the
find_pathfunction insrc/scida/convenience.pyby extracting logic for different backends (HTTP/HTTPS, custom resources, and local datafolders) into separate private helper functions. This simplifies the main function and improves code health.PR created automatically by Jules for task 18189712358400814775 started by @cbyrohl