DM-4639: modernize afw code and reduce doxygen errors #75
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pschella
approved these changes
Apr 25, 2017
There was a problem hiding this comment.
I marked two places as an example, but in general I think auto would be good to use in a lot of places where the type is both not important and the meaning is clear from context (e.g. when an object is returned from a factory or function).
In addition consider scripting usage of make_shared.
| struct Sort_ByX0 : public std::binary_function<typename T::Ptr const, typename T::Ptr const, bool> { | ||
| bool operator() (typename T::Ptr const a, typename T::Ptr const b) const { | ||
| struct Sort_ByX0 : public std::binary_function<std::shared_ptr<T> const, std::shared_ptr<T> const, bool> { | ||
| bool operator() (std::shared_ptr<T> const a, std::shared_ptr<T> const b) const { |
There was a problem hiding this comment.
Do these really need to be owning pointers?
There was a problem hiding this comment.
Maybe not, but converting shared pointers to unique pointers, raw pointers, and/or references is definitely outside the scope of the ticket. For one thing, it would require some more careful thought and testing than the simple syntax replacement I've done here.
| @@ -81,7 +81,7 @@ void PsfImagePca<ImageT>::analyze() | |||
| // Use the median of the edge pixels | |||
|
|
|||
| // If ImageT is a MaskedImage, unpack the Image | |||
| typename afw::image::GetImage<ImageT>::type::Ptr eImageIm = | |||
| std::shared_ptr<typename afw::image::GetImage<ImageT>::type> eImageIm = | |||
| @@ -327,7 +327,7 @@ std::pair<afwMath::LinearCombinationKernel::Ptr, std::vector<double> > createKer | |||
| } | |||
| } | |||
|
|
|||
| kernelList.push_back(afwMath::Kernel::Ptr(new afwMath::FixedKernel( | |||
| kernelList.push_back(std::shared_ptr<afwMath::Kernel>(new afwMath::FixedKernel( | |||
| @@ -458,7 +458,7 @@ class evalChi2Visitor : public afwMath::CandidateVisitor { | |||
| ) : | |||
| afwMath::CandidateVisitor(), | |||
| _chi2(0.0), _kernel(kernel), _lambda(lambda), | |||
| _kImage(KImage::Ptr(new KImage(kernel.getDimensions()))) { | |||
| _kImage(std::shared_ptr<KImage>(new KImage(kernel.getDimensions()))) { | |||
| @@ -778,7 +778,7 @@ class FillABVisitor : public afwMath::CandidateVisitor { | |||
| // | |||
| afwMath::KernelList const& kernels = _kernel.getKernelList(); // Kernel's components | |||
| for (int i = 0; i != _nComponents; ++i) { | |||
| _basisImgs[i] = typename KImage::Ptr(new KImage(kernels[i]->getDimensions())); | |||
| _basisImgs[i] = std::shared_ptr<KImage>(new KImage(kernels[i]->getDimensions())); | |||
| @@ -1120,7 +1120,7 @@ fitKernelParamsToImage( | |||
| afwMath::KernelList newKernels(nKernel); | |||
| std::vector<double> params(nKernel); | |||
| for (int i = 0; i != nKernel; ++i) { | |||
| afwMath::Kernel::Ptr newKernel(new afwMath::FixedKernel(*kernelImages[i])); | |||
| std::shared_ptr<afwMath::Kernel> newKernel(new afwMath::FixedKernel(*kernelImages[i])); | |||
| amp += params[i] * k->getSum(); | ||
| } | ||
|
|
||
| afwMath::Kernel::Ptr outputKernel(new afwMath::LinearCombinationKernel(kernels, params)); | ||
| std::shared_ptr<afwMath::Kernel> outputKernel(new afwMath::LinearCombinationKernel(kernels, params)); |
| @@ -55,7 +55,7 @@ BOOST_AUTO_TEST_CASE(PsfAttributes) { | |||
| int xwid = static_cast<int>(12*sigma0); | |||
| int ywid = xwid; | |||
|
|
|||
| afwDetection::Psf::Ptr psf(new measAlg::SingleGaussianPsf(xwid, ywid, sigma0)); | |||
| std::shared_ptr<afwDetection::Psf> psf(new measAlg::SingleGaussianPsf(xwid, ywid, sigma0)); | |||
src/CR.cc
Outdated
|
|
||
| if (spans.size() > 0) { | ||
| int id = spans[0]->id; | ||
| unsigned int i0 = 0; // initial value of i | ||
| for (unsigned int i = i0; i <= spans.size(); ++i) { // <= size to catch the last object | ||
| if (i == spans.size() || spans[i]->id != id) { | ||
| detection::Footprint::Ptr cr(new detection::Footprint(i - i0)); | ||
| std::shared_ptr<detection::Footprint> cr(new detection::Footprint(i - i0)); |
| @@ -366,7 +366,7 @@ findCosmicRays(MaskedImageT &mimage, ///< Image to search | |||
| * | |||
| * Realise PSF at center of image | |||
| */ | |||
| lsst::afw::math::Kernel::ConstPtr kernel = psf.getLocalKernel(); | |||
| std::shared_ptr<lsst::afw::math::Kernel const> kernel = psf.getLocalKernel(); | |||
kfindeisen
force-pushed
the
tickets/DM-4639
branch
from
b705373
to
279ac61
Compare
kfindeisen
force-pushed
the
tickets/DM-4639
branch
from
279ac61
to
6009397
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DM-4639 requires that
afwcomponents no longer have type membersPtrandConstPtrrepresenting shared pointers to those types. This is a breaking API change that requires changes to downstream code.For clarity, no unnecessary changes changes were made to this package, not even a
clang-formatpass.