Hi you two,

I've put together a wrapper for the prototype L1db that allows it to interface with source association. Yusra AlSayyad, if you could take a look at the ap_assocation code (and the l1db code if you like) that would be great.

Andy Salnikov I've made a few changes to how l1db converts to afw objects and added dependencies for the afw.schema I have in ap_assocaition. I want to make sure that the changes don't adversely effect any of the use cases you have for l1dbproto. I've tested your scripts (create_l1_schema and ap_proto) with the l1db-sqlite.py config file and things seem to run to completion and produce what looks like a reasonable database at the end. Take a look and let me know if anything needs tweaking.

Thanks!

Sorry, in my excitement to finish the ticket I forgot to create pull requests. They are up now.

Hi Chris, 

sorry for delay, swamped with reviews and stuff.

One thing that worries me more than others is that you introduced circular dependency between ap_association and l1dbproto. ap_association needs to depend on l1dbproto of course, but opposite dependency should be avoided. I see that you imported lsst.ap.association into ap_proto and create_l1_schema. I guess you will be needing a script to create schema for ap_association, and I think the easiest is to just copy create_l1_schema from l1dbproto and modify it for your needs. ap_proto does not really need to use exactly the same schema as ap_association so we can probably keep whatever schema it was using before (and I can update that later to match ap_association if needed).

Another thing that I want to check before approving those changes is to verify that degrees to radians conversion works as expected. There may be other locations in ap_proto that depend on units of ra/decl columns, the code may still "work" with different units but it may produce wrong results. Give me day or two to try to run stuff from your branch and see how it behaves.

[Also, if you are ready for merging can you open pull request on github so that we can comment on the code? -- taking this out, took me too long to write my comment]

Cheers,
Andy

Hey Andy,

Thanks for looking at this. I've removed the ap_association dependencies as asked. I mostly had them in there so I could test the association schema in the context of your scripts.

Thanks, Chris! I also pushed couple of small commits  ap_proto had deg<>rad conversion that needed a fix after you switched to Angle in afw table. Also in you last commit imports did not work for me for some reason., had to fix those too. I'll go through PR review later today after all meetings. 

Chris Morrison [X], sorry for delay, I have reviewed both l1dbproto and (very superficially) ap_association changes, check my comments on PR. I think one small unit-related improvement would be useful in l1dbproto. otherwise it all looks OK. I'm removing myself from reviewers, Yusra AlSayyad should mark it as reviewed when done reviewing.

No delay at all Andy Salnikov. Thanks for having a look and glad that most of the changes were reasonable. I'm taking a look through your comments now.

See comments posted in github