Imageomics / dashboard-prototype

Prototype data dashboard for Imageomics Data
http://dash.imageomics.org
MIT License
5 stars 2 forks source link

v1.1.0 dashboard updates #54

Closed egrace479 closed 1 year ago

egrace479 commented 1 year ago

This update brings minor version developments implemented in dev:

  1. Bug fix for lat/lon out of bounds (PR #50, fixes #48).
  2. The size of graphs (and their titles) have been increased for improved visibility.
  3. The map view now has geographic features; base layer of map come from ArcGIS (closes #49).
  4. Removed case-sensitivity of columns and added long as acceptable column name for longitude (PR #51).
    • This introduced a small display bug, which was fixed in PR #52.
  5. Preview images were added to the README to reflect the changes implemented in 2 and 3 above (PR #53).
hlapp commented 1 year ago

Just to clarify for what there is to review, everything in this change set was already reviewed through PRs previously, right? Or are there previously unreviewed changes that review should be focused on?

The change set is a bit big and unwieldy, but if this is just a concatenation of previously reviewed changes, then there's perhaps very little (or even nothing) to review that's new. I.e., trying to figure out what the assignment is for reviewers.

egrace479 commented 1 year ago

Just to clarify for what there is to review, everything in this change set was already reviewed through PRs previously, right? Or are there previously unreviewed changes that review should be focused on?

The change set is a bit big and unwieldy, but if this is just a concatenation of previously reviewed changes, then there's perhaps very little (or even nothing) to review that's new. I.e., trying to figure out what the assignment is for reviewers.

Numbers 2 and 3 (commit c47c550e2214210920256e4b7a922fd4322c7728) were not previously reviewed, but the changes from PRs noted in 4 and 5 were built on top of those changes.

egrace479 commented 1 year ago

file_url had not been adjusted to File_url in the image path call in query or test_query in PR #51 when we adjusted all columns to be capitalized. I just fixed that.

johnbradley commented 1 year ago

I tested with the HCGSD_base_filepath.csv input file and the graph is very zoomed in:

Screen Shot 2023-09-08 at 4 29 15 PM

Running on the main branch the map is properly zoomed out:

Screen Shot 2023-09-08 at 4 29 36 PM

If I zoom out the map in the dev branch it looks more attractive than the main branch version. Is there a way to get the map to automatically zoom like it previously did?

egrace479 commented 1 year ago

I tested with the HCGSD_base_filepath.csv input file and the graph is very zoomed in:

Running on the main branch the map is properly zoomed out:

If I zoom out the map in the dev branch it looks more attractive than the main branch version. Is there a way to get the map to automatically zoom like it previously did?

It seems from this discussion that is a desired, but unimplemented feature. I do have the option to reduce the zoom to--most likely--get points on screen.

This is the zoom set to 2; it will always produce a map of this size centered on the centroid of the lat/longs provided. Screenshot 2023-09-08 at 5 21 48 PM

Zoom set to 3 is more closely cropped, so cuts off for data with a larger spread, but works nicely for data that is more concentrated. Screenshot 2023-09-08 at 5 23 31 PM

I think it makes more sense to err on the side of being more zoomed out (zoom set to 2), though such a large image suggests all data is caught when it may not be. What do you think? (Also, @hlapp and @thompsonmj?)

hlapp commented 1 year ago

I would argue the default should be to zoom out to a level that all data are shown. I think there's a good point that in terms of UX this may mean to zoom out more than strictly necessary if otherwise the view is sufficiently zoomed in to (in this case falsely) give the impression that only some of the data are shown (when really all are).

egrace479 commented 1 year ago

I would argue the default should be to zoom out to a level that all data are shown. I think there's a good point that in terms of UX this may mean to zoom out more than strictly necessary if otherwise the view is sufficiently zoomed in to (in this case falsely) give the impression that only some of the data are shown (when really all are).

Zoom set to 1 would work unless you have data on opposite sides of the earth: Screenshot 2023-09-08 at 6 08 10 PM

Zoom set to 0 could get duplicative, but will definitively show everything: Screenshot 2023-09-08 at 6 09 21 PM

egrace479 commented 1 year ago

Updated zoom to 1 with a note about the potential need to zoom to see all points.

Screenshot 2023-09-11 at 3 23 59 PM