Add module to find ER catalog #161
Conversation
taldcroft
changed the title
taldcroft
force-pushed
the
find-er-catalog
branch
from
19b3a74 to
0a3a7aa
Compare
|
@jeanconn - I'd like to get this in soon so please have a look. It will be better to have it in the release and we can test or develop against it. We can consider this to be experimental for the moment and it's certainly possible there will be API changes when it gets connected with ORviewer. |
|
When you say "the release" do you mean the flight one for today?
…On Wed, Sep 22, 2021, 7:05 AM Tom Aldcroft ***@***.***> wrote:
@jeanconn <https://github.com/jeanconn> - I'd like to get this in soon so
please have a look. It will be better to have it in the release and we can
test or develop against it. We can consider this to be experimental for the
moment and it's certainly possible there will be API changes when it gets
connected with ORviewer.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#161 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAGBHVBIBZPFOQUKXLAIQRLUDG2BBANCNFSM5CZ67HGQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
|
No, I just meant the flight release so it isn't just hanging here as a PR for too long. So whenever the next available release goes out. |
|
Right, the next one is just technically today. But I would like a little more time to review this. |
|
I'm using the Polish next, which is the one after the US English next one. |
| """ | ||
| yags, zags = radec_to_yagzag(stars['RA_PMCORR'], stars['DEC_PMCORR'], att) | ||
| rows, cols = yagzag_to_pixels(yags, zags, allow_bad=True) | ||
| ok = (np.abs(rows) < 507) & (np.abs(cols) < 507) # FIXME: Hardcoded, get from characteristics? |
There was a problem hiding this comment.
Perhaps could use max_ccd_row / col from characteristics, but it isn't like this is going to change much.
| from sparkles.find_er_catalog import ( | ||
| get_candidate_stars, find_er_catalog, filter_candidate_stars_on_ccd, | ||
| get_guide_counts, init_quat_from_attitude) | ||
|
|
There was a problem hiding this comment.
I assume this also needs something like the os.environ[agasc.SUPPLEMENT_ENABLED_ENV] = 'False' that is used in test_review.py .
There was a problem hiding this comment.
Though only a problem if you try to run just the tests in this file.
There was a problem hiding this comment.
You're right, I'm being sloppy. It looks like that env var gets set globally during test collection. This needs to be done with setup/teardown or monkeypatch fixtures. Of course in practice it is fine for testing here to disable globally, but the pattern I had in mind for disabling just in a test module doesn't work. :frown:
There was a problem hiding this comment.
I think "sloppy" is a little harsh, but yes, handling of the env var the way we've used it so far can result in side effects that should be considered.
Description
Add module to find ER catalog.
This requires sot/Quaternion#39.
Testing
Fixes https://github.com/sot/ska-projects/issues/107