Skip to content

Comments

adding hack for lco bug fix#78

Merged
havok2063 merged 2 commits intomainfrom
lcofix
Apr 3, 2025
Merged

adding hack for lco bug fix#78
havok2063 merged 2 commits intomainfrom
lcofix

Conversation

@havok2063
Copy link
Contributor

This PR adds a hack to remove LCO observations from query results. This tacks on an additional where clause to the append_pipes method which filters on the astra fields obs=apo and release=sdss5. This still includes LCO observations from DR17 processed by Astra. The append_pipes method already includes a filter applying the MJD cutoff for a given release. It currently gets used in the following endpoints:

  • main search form (UI)
  • cone search
  • carton-program search
  • target alternate id search
  • target sdss id search

This is some but not all of the endpoints. @Sean-Morrison can you take a look at this? Is this a sufficient enough query to filter out the errant data? And are these endpoints sufficient?

@havok2063 havok2063 added the bug Something isn't working label Apr 1, 2025
@havok2063 havok2063 self-assigned this Apr 1, 2025
@Sean-Morrison
Copy link

At first look, the filter seems a bit simple since DR17 reduced LCO spectra are also included for DR19 Astra, and I think the hack would exclude these.

Additionally, those endpoints seem adequate for the Zora interface, but if we release the API, then it seems like people could use functions like get_targets_obs to circumvent the hack.

@havok2063
Copy link
Contributor Author

ahh yeah you're right about the DR17 LCO observations. This misses those. Yes you're right about the API. I was hoping not to have to worry about it, but I guess it's better to be complete. I'll need to change how this gets implemented then to apply to every endpoint.

@havok2063 havok2063 marked this pull request as ready for review April 2, 2025 15:32
@havok2063
Copy link
Contributor Author

havok2063 commented Apr 2, 2025

@Sean-Morrison I've update the code so the lco hack now applies to the api target/id, target/spectra, and target/pipelines endpoints. The API still returns LCO DR17 observations. The UI would display something like the attached image. One thing is that source catalog and carton info is still displayed on those tabs. Do you think we need to hide this information as well?

Screenshot 2025-04-02 at 11 24 07 AM

@havok2063 havok2063 requested a review from Sean-Morrison April 2, 2025 15:36
Copy link

@Sean-Morrison Sean-Morrison left a comment

Choose a reason for hiding this comment

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

This looks good to me.

@havok2063 havok2063 merged commit b829883 into main Apr 3, 2025
2 checks passed
@havok2063 havok2063 deleted the lcofix branch October 2, 2025 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants