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

Remove dependence on Image_filename for image selector #62

Merged
merged 7 commits into from
Apr 29, 2024
Merged

Conversation

egrace479
Copy link
Member

Displaying sample images required both image_filename and file_url columns. This was an unnecessary complication that is removed in this PR.

Also prints a more specific message when no images are displayed (indicating there are no such entries vs. there are no displayable images for those X entries). This isn't a change in options, just a change in the message itself for greater clarity.

@egrace479
Copy link
Member Author

egrace479 commented Apr 25, 2024

I also noticed a bug in the dropdown selector: if you go from specifying subspecies to selecting Any-<species> for the subspecies, then it will not recognize the Any, as it is returned at this stage as a list (['Any']) instead of a string ('Any'), which is the initial return. This is a result of the check for type that was inserted (at line 142 in the updated components/query.py) to address instances of people not removing the "Any" condition when selecting more subspecies (assuming they'd want the specified options). This is something I intend to fix and update in another PR to dev for further testing before updating main and generating a new release.

Copy link
Contributor

@johnbradley johnbradley left a comment

Choose a reason for hiding this comment

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

LGTM, just one spelling fix.

components/query.py Outdated Show resolved Hide resolved
Copy link
Contributor

@thompsonmj thompsonmj left a comment

Choose a reason for hiding this comment

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

LGTM with a few EOF newlines

test_data/HCGSD_full_testNA.csv Outdated Show resolved Hide resolved
test_data/HCGSD_test_latLonOOB.csv Outdated Show resolved Hide resolved
test_data/HCGSD_test_latLong.csv Outdated Show resolved Hide resolved
test_data/HCGSD_test_nolon.csv Outdated Show resolved Hide resolved
egrace479 and others added 2 commits April 29, 2024 11:45
Add EOF newlines to CSVs

Co-authored-by: Matt Thompson <[email protected]>
Copy link
Member

@hlapp hlapp left a comment

Choose a reason for hiding this comment

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

See one nitpicking suggestion :-)

components/query.py Outdated Show resolved Hide resolved
@egrace479 egrace479 merged commit c4ba3d2 into dev Apr 29, 2024
2 checks passed
@egrace479 egrace479 deleted the url-base branch April 29, 2024 20:42
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.

4 participants