DM-31026: Create Ephemeris Cache Precomputation Task for SSO attribution pipeline #124
Conversation
morriscb
force-pushed
the
tickets/DM-31026
branch
from
1e63b84
to
adb6e7f
Compare
There was a problem hiding this comment.
The fact that requests.get blocks is very concerning. We need to ensure that our pipeline doesn't get stuck waiting for data that may never come. We may need to use a different library than requests to make it asynchronous or have a timeout.
I also think we need more tests of potential failure modes of the get.
tests/test_ephemerisQuery.py
Outdated
| import lsst.utils.tests | ||
|
|
||
|
|
||
| class TestEphemerisQuery(unittest.TestCase): |
There was a problem hiding this comment.
There are a few different requests mock/test libraries such as requests-mock; I suggest using one of those to write some tests that include failure conditions (403/404 errors, data dropouts). I suspect #dm-square will have suggestions about what libraries to use, and how to implement such tests.
There was a problem hiding this comment.
I'll address it as above with the other change to requests ticket.
Bugfix Bugfix Conversion of ephemerisChache task to pipe task. Clean up. Updated after paircoding with Chris Morrison. Add connections and loop over difference images. Debug pipeline for smoke test run. Skybot cone search added. Change tabs to spaces Fix linting. Fix linting Debug pipeline. Fix typos
Clean up ssoAssociation code. Change output name and butler dimenions. Clean up ssoAssociation add test_ssoAssociation Modify config and dimensions. Fix import Debug pipeline and fully test integration.
Update dimenions. Debug Writing associated diaSources. Remove write statement.
Debug SSO association unittest.
Fix linting.
morriscb
force-pushed
the
tickets/DM-31026
branch
from
adb6e7f
to
0307648
Compare
| system object caching/retrieval code. | ||
|
|
||
| Will compute the location for of known SSObjects within a visit. Currently uses | ||
| an external web service retrieve data and is meant to be used as a stand alone |
There was a problem hiding this comment.
external web service *to* retrieve data?
|
|
||
| Will compute the location for of known SSObjects within a visit. Currently uses | ||
| an external web service retrieve data and is meant to be used as a stand alone | ||
| pipeline. Use in a larger pipeline at your own risk. |
There was a problem hiding this comment.
I'd be even stronger about this: "This code blocks on web requests, so should not be used as part of any realtime or time-sensitive system."
|
|
||
| def runQuantum(self, butlerQC, inputRefs, outputRefs): | ||
| inputs = butlerQC.get(inputRefs) | ||
| inputs["visit"] = butlerQC.quantum.dataId["visit"] |
There was a problem hiding this comment.
This is something I want to clear up with the middleware team at some point. For now, you don't need to do anything here.
| orbitUncertaintyFilter = queryRadius | ||
|
|
||
| q = ['http://vo.imcce.fr/webservices/skybot/skybotconesearch_query.php?'] | ||
| q.append('-ra=' + str(fieldRA)) |
There was a problem hiding this comment.
Please note the ticket number in a TODO: here.
| return dfSSO | ||
|
|
||
| def _decdms2decdeg(self, decdms): | ||
| """Convert Declination from degrees minutes seconds to decimal degrees. |
There was a problem hiding this comment.
See the creation examples here, which include exactly that: https://docs.astropy.org/en/stable/coordinates/index.html#example
Then you can do cc.dec.to_value(u.deg) and cc.ra.to_value(u.deg).
| result = requests.request("GET", query) | ||
| dfSSO = pd.read_csv(StringIO(result.text), sep='|', skiprows=2) | ||
| if len(dfSSO) > 0: | ||
| columns = [col.strip() for col in dfSSO.columns] |
There was a problem hiding this comment.
Probably worth a comment, for those not familiar with the format of this data.
Alternately: is it readable by astropy.tables? That is a way to get it straight into the right data types.
morriscb
force-pushed
the
tickets/DM-31026
branch
3 times, most recently
from
25227ff
to
01dc450
Compare
|
You can at least squash the post-review commits into their appropriate other commits. |
Change doc string units. Update license. Second response to reviewer. Fix linting. Change name to SkyBotEphemerisQuery Fix linting. Fix linting.
morriscb
force-pushed
the
tickets/DM-31026
branch
from
01dc450
to
8a66b59
Compare
No description provided.