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.

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

Fix and refactor NumberTraits #1397

Merged
merged 4 commits into from
Mar 14, 2019
Merged

Conversation

rolanddenis
Copy link
Member

@rolanddenis rolanddenis commented Feb 22, 2019

PR Description

Fix issues with NumberTraits (see #1396) and refactor this class by relying on std::numeric_limits when possible (i.e. for all fundamental arithmetical types).

Also add a test file for NumberTraits.

Checklist

  • Unit-test of your feature with Catch.
  • Doxygen documentation of the code completed (classes, methods, types, members...)
  • New entry in the ChangeLog.md added.
  • No warning raised in Debug cmake mode (otherwise, Travis C.I. will fail).
  • All continuous integration tests pass (Travis & appveyor)

And adding static properties definition for currently non-specialized
types (long long int).
So that to have associated documentation generated.
It now relies mainly on std::numeric_limits for fundamental types so
that we don't need a specialization per type anymore.

Also fix some wrong type traits (digits of floating-point types and
                                 BigInteger bound status).
Also remove an unwanted quote.
@dcoeurjo
Copy link
Member

dcoeurjo commented Mar 5, 2019

thanks @rolanddenis .. reviewing ;)

@dcoeurjo
Copy link
Member

well you're way more skilled than me for this kind of type related stuffs...

@dcoeurjo dcoeurjo self-requested a review March 14, 2019 16:33
@dcoeurjo
Copy link
Member

Let's merge this one;) thx

@dcoeurjo dcoeurjo merged commit cceb3a6 into DGtal-team:master Mar 14, 2019
@rolanddenis rolanddenis deleted the fix_numbertraits branch March 14, 2019 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants