DM-9135 bulk rename of non-conforming jointcal variables/methods #42
Conversation
parejkoj
force-pushed
the
tickets/DM-9135
branch
2 times, most recently
from
566b17c
to
0ae5ebd
Compare
|
|
||
| virtual ~BaseStar(){}; | ||
|
|
||
| double getFlux() const { return _flux; } | ||
| double &getFlux() { return _flux; } |
There was a problem hiding this comment.
Yes: PhotometryFit has fs.getFlux() += delta(index);
include/lsst/jointcal/CcdImage.h
Outdated
| @@ -51,9 +51,8 @@ class CcdImage { | |||
| VisitIdType _visit; | |||
|
|
|||
| lsst::afw::coord::IcrsCoord boresightRaDec; | |||
| double airMass; // airmass value. | |||
| double fluxCoeff; // coefficient to convert ADUs to ADUs/sec at airmass 1 | |||
There was a problem hiding this comment.
It wasn't used anywhere. Flux conversions are really the job of PhotoCalib, and I'll use that for such things in the future.
include/lsst/jointcal/CcdImage.h
Outdated
| @@ -146,34 +145,31 @@ class CcdImage { | |||
| lsst::afw::coord::IcrsCoord getBoresightRaDec() { return boresightRaDec; } | |||
|
|
|||
| //! | |||
| double HourAngle() const { return hourAngle; } | |||
| double getHourAngle() const { return hourAngle; } | |||
There was a problem hiding this comment.
No underscores (here and below)? These are private members?
There was a problem hiding this comment.
Good catch: I've underscored all the private members in ccdImage.h.
| //! | ||
| int& MeasurementCount() { return measurementCount; } | ||
| int getMeasurementCount() const { return _measurementCount; } | ||
| int& getMeasurementCount() { return _measurementCount; } |
There was a problem hiding this comment.
Yes, it's used to increment and decrement measurementCount when constructing and cleaning the star lists.
include/lsst/jointcal/Frame.h
Outdated
|
|
||
| //! same as above | ||
| bool InFrame(const Point &pt) const { return InFrame(pt.x, pt.y); } | ||
| bool inFrame(const Point &pt) const { return inFrame(pt.x, pt.y); } |
include/lsst/jointcal/Frame.h
Outdated
|
|
||
| //! distance to closest boundary. | ||
| double MinDistToEdges(const Point &P) const; | ||
| double minDistToEdges(const Point &P) const; |
| fs->MeasurementCount()--; // could be put in SetValid | ||
| auto fs = std::const_pointer_cast<FittedStar>(ms.getFittedStar()); | ||
| ms.setValid(false); | ||
| fs->getMeasurementCount()--; // could be put in setValid |
There was a problem hiding this comment.
Because I'm not sure I believe the comment, but I'm also not sure I don't (which is why I haven't deleted it).
src/Gtransfo.cc
Outdated
| double GtransfoPoly::do_the_fit(const StarMatchList &List, const Gtransfo &ShiftToCenter, | ||
| const bool UseErrors) { | ||
| Eigen::MatrixXd A(2 * nterms, 2 * nterms); | ||
| double GtransfoPoly::do_the_fit(const StarMatchList &starMatchList, const Gtransfo &shiftToCenter, |
There was a problem hiding this comment.
Please also check for other snake_case_functions and variables.
There was a problem hiding this comment.
I just went through with a regex, and I think I got them all. Thanks.
src/ListMatch.cc
Outdated
| deltaSizeRatio = 0.1 * sizeRatio; | ||
| minMatchRatio = 1. / 3.; | ||
| printLevel = 0; | ||
| algorithm = 2; |
There was a problem hiding this comment.
These should really go in the intializer list, but again another ticket (cleaning up constructors).
There was a problem hiding this comment.
Just moved it into the initializer list, and put it in the header instead. Was easy enough to do.
src/ListMatch.cc
Outdated
| ++it; | ||
| } | ||
| } | ||
| // double dx=0, dy=0; | ||
| // histo.MaxBin(dx,dy); | ||
| // histo.maxBin(dx,dy); | ||
| // return new GtransfoLinShift(dx,dy); |
src/ListMatch.cc
Outdated
| // BaseStar *neighbour = finder.FindClosest(p2,MaxDist); | ||
| const Point p2 = guess->apply(*p1); | ||
| const BaseStar *neighbour = list2.findClosest(p2); | ||
| // BaseStar *neighbour = finder.findClosest(p2,maxDist); |
Cleanup getters per discussion on ticket Cleanup silly compiler warning squashing post-renaming clang-format pass
|
Thanks for the review. I've responded to, or taken care of all your comments, and have rebase-squashed all the changes into one. |
No description provided.