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.

Sign up for GitHub

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

Jump to bottom

Tickets/dm 5933 #13

Merged
merged 4 commits into from
Jul 8, 2016
Merged

Tickets/dm 5933 #13

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8348807
Remove unnecessary couts
parejkoj Jul 4, 2016
76a17a1
Change default field name so it must be set to something.
parejkoj Jul 5, 2016
7c78f95
Get jointcal to work with new astrometrySourceSelector.
parejkoj Jul 5, 2016
6a7799b
Don't need slot_Centroid_flag: astrometrySourceSelector takes care of it
parejkoj Jul 8, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/lsst/jointcal/Jointcal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ namespace jointcal {
LSST_CONTROL_FIELD(sourceFluxField, std::string, "name of flux field in source catalog");

JointcalControl() :
sourceFluxField("base_CircularApertureFlux_7")
sourceFluxField("None")
Copy link

Choose a reason for hiding this comment

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

I'm not convinced this is doing what you think it is doing, or that perhaps I don't understand something about LSST_CONTROL_FIELDS. You are setting it to "None" so that a user must specify a parameter. However "None" is still a string, and thus validate() will still succeed and the program will fail later when it tries to find the None in the schema. Wouldn't it be better to leave the control field blank, so that validate the control early, and have it throw a message about needing to set it? Again, maybe I am just not following what the control field is doing and "None" has a special meaning here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmmm... None doesn't have special meaning, I just wanted to set it to something that was clearly not a valid field. Previously, jointcal was cout<<'ing the above value somewhere further down the line even though I set a different value for sourceFluxField later on. I made this change to ensure it wasn't pulling something else out of the schema. Validate only checks that the field isn't empty, nothing more.

I can't set it to a blank string because validate() will fail during instantiation.

I want to totally remove sourceFluxField in the future: I don't think we actually need it, but tearing it out is not going to be easy.

{
validate();
}
Expand Down