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
DM-15208 Fix disabled skipTEx option #85
Conversation
e51e047
to
da6aaec
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In some cases I confess I couldn't really see what was going on, but overall it looked fine. I had a few minor questions and suggestions.
@@ -46,7 +44,7 @@ | |||
|
|||
|
|||
def build_matched_dataset(repo, dataIds, matchRadius=None, safeSnr=50., | |||
useJointCal=False): | |||
useJointCal=False, skipTEx=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you really want to force skipTEx to False here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default for it is False everywhere, which is changed by the config option.
afx_spec_set = specs.subset(required_meta={'instrument':'HSC'}, spec_tags=[afxName,]) | ||
adx_spec_set = specs.subset(required_meta={'instrument':'HSC'}, spec_tags=[adxName,]) | ||
afx_spec_set = specs.subset(required_meta={'instrument': 'HSC'}, spec_tags=[afxName, ]) | ||
adx_spec_set = specs.subset(required_meta={'instrument': 'HSC'}, spec_tags=[adxName, ]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you really want HSC hard-coded here? The value is instrument
below. I realize this is pre-existing code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not going to modify the existing code other than to flake8 clean it: @SimonKrughoff or @wmwv would have to decide what appropriate defaults are here.
Use jointcal/meas_mosaic outputs to calibrate positions and fluxes. | ||
skipTEx : bool, optional | ||
Skip TEx calculations (useful for older catalogs that don't have | ||
PsfShape measurements). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please put classes in back tickets at least for new docs, e.g. bool
-> `bool`
7728193
to
3930d47
Compare
376a4a7
to
ecaaa75
Compare
No description provided.