Skip to content

1187 aladin skymap blocking load catalog per target#1190

Merged
Fingel merged 4 commits into1186-target-list-page-improvementsfrom
1187-aladin-skymap-blocking-load-catalog-per-target
Mar 5, 2025
Merged

1187 aladin skymap blocking load catalog per target#1190
Fingel merged 4 commits into1186-target-list-page-improvementsfrom
1187-aladin-skymap-blocking-load-catalog-per-target

Conversation

@Fingel
Copy link
Copy Markdown
Contributor

@Fingel Fingel commented Feb 26, 2025

Uses a dedicated JSON endpoint to load targets for the Aladin map, making it efficient enough that all (well, up to 10k) targets that match a filter can be displayed on the map, instead of only the 25 in the current pagination page. This is much more intuitive and what I'd expect to see on the map.

The 10k number can be adjusted but at that point there's so many targets that it's not worth adding more as you won't really be able to see a difference.

This is important because with infinite scroll, it's not possible to keep what is displayed on the Aladin map consistent with the current "page".

Fixes #1187

Screencast.From.2025-02-26.10-35-47.mp4

@Fingel Fingel requested review from jchate6 and phycodurus February 26, 2025 18:39
@jchate6 jchate6 moved this to Needs Review in TOM Toolkit Mar 3, 2025
@jchate6 jchate6 linked an issue Mar 3, 2025 that may be closed by this pull request
Copy link
Copy Markdown
Contributor

@jchate6 jchate6 left a comment

Choose a reason for hiding this comment

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

I have some misgivings about these changes. I feel like we are losing a fair amount of utility for the skymap for the sake of accommodating unnavigable lists of targets.

The ability to manipulate which individual targets are displayed could be very useful when there are only a few.

I understand the 10k is just an arbitrary choice, but it's tiny for a full TOM DB. Is there an actual upper limit where performance is noticeably affected? It feels like we shouldn't have a warning as the default status for the page most TOMs use as their defacto home page.


# If the queryset is too large, take a random sample
if target_count >= 10_000:
qs = qs.order_by('?')[:10_000]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd argue the most recently added would be better than random.

@jchate6
Copy link
Copy Markdown
Contributor

jchate6 commented Mar 3, 2025

I'm happy to merge this into 1186 and see how users feel when everything is together.

@Fingel
Copy link
Copy Markdown
Contributor Author

Fingel commented Mar 3, 2025

I have some misgivings about these changes. I feel like we are losing a fair amount of utility for the skymap for the sake of accommodating unnavigable lists of targets.

The ability to manipulate which individual targets are displayed could be very useful when there are only a few.

I understand the 10k is just an arbitrary choice, but it's tiny for a full TOM DB. Is there an actual upper limit where performance is noticeably affected? It feels like we shouldn't have a warning as the default status for the page most TOMs use as their defacto home page.

I'm super curious what functionality we are giving up? There are other (better) ways to hide individual markers, if that's what you need to do: for example, clicking one and choosing "hide".

I think it's strange that the skymap shows 25 targets currently, even if I have 1000's in my database. Why 25? Because that's the number we landed on for pagination? Now that is arbitrary.

In any place where a graph of objects like this appears, I think it's natural to assume that is represents all objects that match your filters, not just an arbitrarily small number of them that happen to map to what is currently shown in the table. Take a cone search for example. I want to find all objects within x arcseconds of some star. Wouldn't it be more natural to see a full representation of them, not just 25? I don't care what page of results I happen to be shown, pages have nothing to do with the question.

10k is an arbitrary upper limit, the actual limit on the map is much higher. Fine with bumping up this number.

@jchate6
Copy link
Copy Markdown
Contributor

jchate6 commented Mar 4, 2025

Good points. I think this is an issue of use cases. This display has different use cases based on circumstance and each TOM will use it differently, but our goal here is to make it flexible enough to meet those varied needs while having a sensible default functionality.

The different use cases that I'm thinking of are as follows:

  1. A default list of targets:

    • This could represent my entire DB or just some filtered/sorted subset of default "interesting" targets.
    • displayed when I first arrive on the page.
    • In either case displaying the FULL list on my plot is not practical. When I first go to this page, seeing the top N (definitely arbitrary from our perspective, but with n<100 ) targets on the list is really what I want. I want to know where in the sky the (north/south, proximity to the sun/moon) the most interesting targets on my list are at a glance. Hiding these targets among a sea of thousands of other targets makes this entire figure useless.
    • By default we assumed that the 25 most recently added targets are the most interesting, and that also fully represented the targets visible on the page when we were using pagination, and the displayed targets would update as the page changed.
  2. A filtered list:

    • A non-default query, showing some subset of targets determined by the filters.
    • Displayed after altering the filter values on the page.
    • Here I agree that if I've performed a specific query, we should return all the targets. I agree that if we do a cone search I should see all the targets in that cone.

I'm super curious what functionality we are giving up? There are other (better) ways to hide individual markers, if that's what you need to do: for example, clicking one and choosing "hide".

It's definitely flawed, but the ability to hide certain targets by name is definitely important functionality. I do not see a way to click on one and choose hide.

I think it's strange that the skymap shows 25 targets currently, even if I have 1000's in my database. Why 25? Because that's the number we landed on for pagination? Now that is arbitrary.

Agreed, but showing all of them effectively removes the utility of the plot. The idea was that displaying the targets on the page at least mapped the page to the plot so you could know exactly what was being displayed.

In any place where a graph of objects like this appears, I think it's natural to assume that is represents all objects that match your filters, not just an arbitrarily small number of them that happen to map to what is currently shown in the table. Take a cone search for example. I want to find all objects within x arcseconds of some star. Wouldn't it be more natural to see a full representation of them, not just 25? I don't care what page of results I happen to be shown, pages have nothing to do with the question.

Agreed. Unless my filter is all.

10k is an arbitrary upper limit, the actual limit on the map is much higher. Fine with bumping up this number.

Does it need to have a limit? Can we give the user the option to display the top N results?

Do those points make sense?

@Fingel
Copy link
Copy Markdown
Contributor Author

Fingel commented Mar 5, 2025

I added a limit filter, which should alleviate many of these issues. For example, only displaying the most recent 25 targets, in both the map and the table, would be achieved by entering 25 into the result limit filter.

@Fingel Fingel merged commit 11ff3f3 into 1186-target-list-page-improvements Mar 5, 2025
20 of 21 checks passed
@github-project-automation github-project-automation bot moved this from Needs Review to Merged (to dev) in TOM Toolkit Mar 5, 2025
@Fingel Fingel deleted the 1187-aladin-skymap-blocking-load-catalog-per-target branch March 5, 2025 23:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Aladin Skymap blocking load, catalog per-target.

2 participants