Drop unnecessary moving window filter pimpl

[mjcarroll]

Came across this as I was deprecating in ign-common. The pimpl pattern doesn't make sense for a templated class since the implementation is ultimately exposed to downstream users.

This breaks API/ABI, so I'm targeting main with it.

Note to maintainers: Remember to use Squash-Merge

[jwnimmer-tri]

This touches upon a point that I was going to file an issue about later on -- when the math classes here are templated on <T>, is that only in support of choosing either float or double for the scalar type, or is the set of allowed types open to the user to expand?

The documentation isn't clear on what types are permitted to use for T. The code as-written does not seem sufficiently general to allow for user-provided floating point numbers. (It would need to use either more ADL or Eigen's NumTraits, if more types were wanted.)

Assuming that the library only supports C++'s native floating-point types, then I don't see any reason to place the function definitions in the header files. The implementation could move into the cc files by using explicit template instantiation over the set of supported template arguments.

[mjcarroll]

is that only in support of choosing either float or double for the scalar type, or is the set of allowed types open to the user to expand?

I think that's a good point. I wasn't around for the earlier iterations of ign-math, so I'm not sure what the intent was. I know that there is at least support for int, float, and double for Vector, but that I don't believe that int would work here with the way it is implemented.

Assuming that the library only supports C++'s native floating-point types, then I don't see any reason to place the function definitions in the header files. The implementation could move into the cc files by using explicit template instantiation over the set of supported template arguments.

I think this would be a reasonable approach.

Perhaps @nkoenig or @chapulina could lend some insight?

[chapulina]

I don't have a lot of context on this class, I don't remember ever using it, and I couldn't find any code that uses it. I traced it back to this commit on Gazebo classic: gazebosim/gazebo-classic@16e1af9

The implementation could move into the cc files by using explicit template instantiation over the set of supported template arguments.

I think that's ok. There'd be a bit of duplication, but the class is small enough...

[mjcarroll]

Added explicit instantiation for:

template class MovingWindowFilter<int>;
template class MovingWindowFilter<float>;
template class MovingWindowFilter<double>;
template class MovingWindowFilter<ignition::math::Vector3i>;
template class MovingWindowFilter<ignition::math::Vector3f>;
template class MovingWindowFilter<ignition::math::Vector3d>;

[codecov]

Codecov Report

Merging #294 (b1c6b93) into main (3a2147c) will decrease coverage by 0.15%.
The diff coverage is 76.74%.

❗ Current head b1c6b93 differs from pull request most recent head 13c80a7. Consider uploading reports for the commit 13c80a7 to get more accurate results

@@            Coverage Diff             @@
##             main     #294      +/-   ##
==========================================
- Coverage   99.73%   99.57%   -0.16%     
==========================================
  Files          70       71       +1     
  Lines        6406     6412       +6     
==========================================
- Hits         6389     6385       -4     
- Misses         17       27      +10     

Impacted Files	Coverage Δ
src/MovingWindowFilter.cc	76.19% <76.19%> (ø)
include/ignition/math/MovingWindowFilter.hh	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a2147c...13c80a7. Read the comment docs.

[mjcarroll]

Blocked on #299

[nkoenig]

I don't have additional context on this class. This looks fine.

[codecov]

Codecov Report

Merging #294 (ac32fcc) into main (b03f97c) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #294   +/-   ##
=======================================
  Coverage   99.69%   99.69%           
=======================================
  Files          74       75    +1     
  Lines        6857     6863    +6     
=======================================
+ Hits         6836     6842    +6     
  Misses         21       21           

Impacted Files	Coverage Δ
include/gz/math/MovingWindowFilter.hh	100.00% <100.00%> (ø)
src/MovingWindowFilter.cc	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b03f97c...ac32fcc. Read the comment docs.

[scpeters]

looks good with clean CI