-
Notifications
You must be signed in to change notification settings - Fork 5
Advanced search filters #1
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
base: main
Are you sure you want to change the base?
Conversation
…l as safesearch-setting functionality.
Hey, I'm pretty busy this week so I probably won't have time to review/merge until the weekend. Great idea! At first glance, code looks good. Thanks for following the style :) |
2aaf2eb
to
767c8a8
Compare
There's 2 big issues that have to be fixed (I'll help) 1.) The filters should all be optional 2.) Each option needs to be tested. This includes all of the choices for categorical arguments (large, medium, tall, square, etc) 3.) Plz try to avoid rows longer than 80 columns |
src/scraper.py
Outdated
@@ -174,7 +259,8 @@ def scrape_images(search_key, image_cnt, directory, threads): | |||
print("savedir: {}".format(directory)) | |||
if not os.path.exists(directory): | |||
os.makedirs(directory) | |||
|
|||
global search_url |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The global should be called in setup_url so the logic is compartmentalized
src/scraper.py
Outdated
return key + 'ol' | ||
else: | ||
return '' | ||
|
||
def process_safesearch(val: str): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the default safesearch behavior any different from "off"?
src/scraper.py
Outdated
@@ -122,7 +147,7 @@ def setup_url(searchurl: str, imgsize: str, imgcolor: str, imgtype: str, safesea | |||
features += ["tbs=" + delim2.join(subfeatures[0])] | |||
if (subfeatures[1] != []): | |||
features += ["safe=" + delim2.join(subfeatures[1])] | |||
|
|||
print(delim1.join(features)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please wrap debugging print statements in an 'if DEBUG' and provide them useful text to identify the output
src/main.py
Outdated
@@ -5,7 +5,7 @@ | |||
|
|||
def main(): | |||
args = get_arguments(sys.argv) | |||
scrape_images(args.keyword[0], args.count, args.directory, args.threads, args.size, args.color, args.imgtype, args.safesearch) | |||
scrape_images(args.keyword[0], args.count, args.directory, args.threads, args.size, args.aspectratio, args.color, args.imgtype, args.region, args.filetype, args.usage, args.safesearch) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filters should be passed as a single object so we can try to avoid these long definitions that look ugly. IMO if you can't call the function on one line, it needs to be refactored into a more compact structure
Sorry, haven't revised/added anything, but realized that this can't be merged until it's "marked as ready". |
Added a bunch of arguments corresponding to the various advanced image search filters and the safesearch setting.