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

Rework photometry filters #379

Merged

Conversation

dr-rodriguez
Copy link
Collaborator

@dr-rodriguez dr-rodriguez commented Jun 28, 2023

This updates the Photometry and PhotometryFilters table in various ways:

  • UCD column moved from Photometry to PhotometryFilters
  • instrument and telescope columns removed from PhotometryFilters
  • Foreign Key of Photometry reworked to point to Telescopes
  • instrument column removed from Photometry (in anticipation of Rework Instruments table #380)

Closes #293

@dr-rodriguez dr-rodriguez marked this pull request as ready for review June 28, 2023 18:18
@dr-rodriguez dr-rodriguez requested a review from kelle June 28, 2023 18:33
@kelle
Copy link
Collaborator

kelle commented Jun 30, 2023

This looks good but I admit to not having my head fully wrapped around it. Should we write some tests to go along with it?

@dr-rodriguez
Copy link
Collaborator Author

We have tests that query Photometry and PhotometryFilters already, I'm not sure what else we would add to them. The database will not build if the foreign keys and similar are not set.

I did notice the tests failing with the Instruments change (#380), but that's a separate PR.

@kelle
Copy link
Collaborator

kelle commented Jul 11, 2023

This change will break the ingest_photometry function. The tests still pass because there are currently no tests for that function. I will open an issue to write tests and modify the ingest_photometry function accordingly.

@kelle kelle mentioned this pull request Jul 11, 2023
@dr-rodriguez dr-rodriguez merged commit 5a6174f into SIMPLE-AstroDB:main Jul 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tweak PhotometryFilters table
2 participants