Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add glob::glob file expansion #134

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

acesnik
Copy link

@acesnik acesnik commented Apr 30, 2024

@acesnik acesnik marked this pull request as draft April 30, 2024 22:11
@acesnik acesnik changed the title Add file expansion and file existance checks Add glob::glob file expansion Apr 30, 2024
@acesnik acesnik marked this pull request as ready for review May 2, 2024 22:10
@acesnik
Copy link
Author

acesnik commented May 2, 2024

I tested this out on a simple bash commandline.

  • It works for pattern matching mzMLs, e.g. *.mzML.
  • When nothing matches, it returns Error: mzml_paths must be set. For more information try '--help'.
  • With a malformed glob, e.g. L[*.mzML, it returns Glob pattern error: Pattern syntax error near position 1: invalid range pattern. This might create some undesirable behavior, treating every path as a glob request... What do you think?

Copy link
Owner

@lazear lazear left a comment

Choose a reason for hiding this comment

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

Instead of eprintln!, can you use log::error!? That will give us consistent output with the rest of the program, and enable filtering of logs

@acesnik
Copy link
Author

acesnik commented May 4, 2024

Yep, sounds good and made the change. :)

@lazear
Copy link
Owner

lazear commented May 8, 2024

After some quick testing, this appears to break s3 support if the url is specified via CLI - IMO the best way to do this will be to move this logic into the sage-cloudpath crate for now. This would also enable glob paths to work when specified via the JSON configuration file.

We could then implement a method on the CloudPath enum that handles globs for both S3 (via list-objects) and local files (via glob)

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.

None yet

2 participants