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

133 populate adlerdata from database #162

Merged
merged 3 commits into from
Aug 27, 2024
Merged

Conversation

astronomerritt
Copy link
Collaborator

Fixes #133.

  • The AdlerData object can now also populate itself from a SQL database, in which case it reads the most recently timestamped entry for its ssObjectId, pulls all data associated with the filter list you specify, and sorts it into the right attributes. This can be done explicitly on an AdlerData object by calling AdlerData.populate_from_database(<filepath>).
  • Previously calculated AdlerData can be attached to an AdlerPlanetoid object by calling AdlerPlanetoid.attach_previous_adler_data(<filepath>). This creates self.PreviousAdlerData, an AdlerData object populated with the most recent AdlerData for the ssObjectId.
  • Wrote unit tests.
  • Made a little notebook to explain how it works.

Review Checklist for Source Code Changes

  • Does pip install still work?
  • Have you written a unit test for any new functions?
  • Do all the units tests run successfully?
  • Does adler run successfully on a test set of input files/databases?
  • Have you used black on the files you have updated to confirm python programming style guide enforcement?

@astronomerritt astronomerritt requested a review from jrob93 August 20, 2024 16:56
@jrob93
Copy link
Collaborator

jrob93 commented Aug 22, 2024

Hey this is great thanks. I'm playing around with it now but here's some initial thoughts. Currently write_row_to_database appends a timestamped row, so we can have multiple entries for each object and timestamp is the unique identifier. This is certainly useful, but would it be useful to also have an adlerData_id long int unique identifier or similar? Probably a question for Ken in terms of efficiently indexing a database and if there are any condsiderations on the data type of a column you index on.

Also, when we get larger we will probably prefer to overwrite a single entry for each object rather than appending and letting things grow. Should we have a wrapper function for something like update_row_to_database?

@astronomerritt
Copy link
Collaborator Author

Good points both. I can see the utility of an AdlerID for sure, I'll add it to my list of things to ask Ken when I get hold of him.

And yeah, I expect we'll be looking to overwrite single entries for the final AdlerData database when it comes to the Lasair end of things - I think we discussed this previously. I'll add that functionality at some point. Making tickets for both of these, thanks!

Copy link
Collaborator

@jrob93 jrob93 left a comment

Choose a reason for hiding this comment

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

Sorry for not finishing this review earlier. I think this all looks good to me. My only comments are the ones made above, but this PR as is certainly fulfils what we need to do now

@astronomerritt astronomerritt merged commit 4ce7bb3 into main Aug 27, 2024
10 checks passed
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.

AdlerPlanetoid should also ingest from saved AdlerData
2 participants