DM-52986: Add wrapper for SplineMap #70
Conversation
include/astshim/SplineMap.h
Outdated
| * | ||
| * You should have received a copy of the LSST License Statement and | ||
| * the GNU General Public License along with this program. If not, | ||
| * see <https://www.lsstcorp.org/LegalNotices/>. |
There was a problem hiding this comment.
The license banner is outdated
| * You should have received a copy of the LSST License Statement and | ||
| * the GNU General Public License along with this program. If not, | ||
| * see <https://www.lsstcorp.org/LegalNotices/>. | ||
| */ |
There was a problem hiding this comment.
I fixed these two licenses, plus the python/astshim/splineMap.cc one, but it looks like almost all the others in the repo are wrong too. I guess that can be another ticket.
include/astshim/SplineMap.h
Outdated
| : Mapping(reinterpret_cast<AstMapping *>( | ||
| _makeRawSplineMap(kx, ky, nx, ny, tx, ty, cu, cv, options))) {} | ||
|
|
||
| virtual ~SplineMap() {} |
There was a problem hiding this comment.
| virtual ~SplineMap() {} | |
| ~SplineMap() override = default; |
include/astshim/SplineMap.h
Outdated
|
|
||
|
|
||
| protected: | ||
| virtual std::shared_ptr<Object> copyPolymorphic() const override { |
There was a problem hiding this comment.
virtual is redundant as the function is declared override
| virtual std::shared_ptr<Object> copyPolymorphic() const override { | |
| std::shared_ptr<Object> copyPolymorphic() const override { |
include/astshim/SplineMap.h
Outdated
| } | ||
|
|
||
| /// Construct a SplineMap from an raw AST pointer | ||
| SplineMap(AstSplineMap *map); |
There was a problem hiding this comment.
| SplineMap(AstSplineMap *map); | |
| explicit SplineMap(AstSplineMap *map); |
include/astshim/SplineMap.h
Outdated
| #ifndef ASTSHIM_SPLINEMAP_H | ||
| #define ASTSHIM_SPLINEMAP_H | ||
|
|
||
| #include <algorithm> |
There was a problem hiding this comment.
Nothing from algorithm is used in this header
| @@ -0,0 +1,51 @@ | |||
| /* | |||
There was a problem hiding this comment.
I think this is the newer banner that should be used in the other new files as well.
There was a problem hiding this comment.
I used the one here: https://developer.lsst.io/stack/license-and-copyright.html#c-preamble
src/SplineMap.cc
Outdated
| std::vector<double> const &ty, std::vector<double> const &cu, std::vector<double> const &cv, | ||
| std::string const &options) const { | ||
|
|
||
| const int nTx = tx.size(); |
There was a problem hiding this comment.
These do a narrowing cast.
size() returns size_t (unsigned 64 bit)
int is 32 bit (signed)
Likely OK as the vectors are not big.
There was a problem hiding this comment.
To be safe, I changed these to size_t, added a check that kx, ky, nx, and ny are not negative, and convert the kx + nx, nx * ny, etc. terms to size_t to prevent a compiler warning.
mwittgen
left a comment
mwittgen
left a comment
There was a problem hiding this comment.
Some minor comments.
Check license banners.
cmsaunders
force-pushed
the
tickets/DM-52986
branch
from
a448c93 to
7dca3bd
Compare
No description provided.