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

Draft: Get rid of deprecation warnings #45

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

nordlow
Copy link

@nordlow nordlow commented Sep 3, 2021

Gets rid of deprecation warnings via:

  1. body => do
  2. rename variable to avoid shadow warning
  3. approxEqual => isClose: I started updating all those darn floating point literals to a better precision to fit isClose's requirements but I got fed up with it because it wasted a lot of time and I couldn't find a way to automated it. So I added an approxEqual that mimics old deprecated version to make tests pass without warnings nor unittest failures. But kept the constants refactorings. I can continue updating the rest of literals if needed but I'd rather not. :)
  4. git ignore dstats-test-library

Ready for review now, @John-Colvin .

For reference see https://dlang.org/changelog/2.096.0.html#deprecate-approxEqual.

@nordlow nordlow changed the title Draft: Replace deprecated approxEqual with isClose Draft: Get rid of deprecation warnings Sep 3, 2021
@nordlow nordlow changed the title Draft: Get rid of deprecation warnings Get rid of deprecation warnings Sep 3, 2021
source/dstats/distrib.d Outdated Show resolved Hide resolved
@nordlow
Copy link
Author

nordlow commented Sep 4, 2021

Shall I move

static if (__VERSION__ < 2096)
        alias approxEqual = std.math.approxEqual;
    else
        bool approxEqual(T, U, V)(T lhs, U rhs, V maxRelDiff = 1e-2, V maxAbsDiff = 1e-5)
        {
            return std.math.isClose(lhs, rhs, maxRelDiff, maxAbsDiff); // mimic old sloppy approxEqual for now
        }

to a common place? Say, base.d?

@skoppe
Copy link

skoppe commented Sep 4, 2021

What I understand from this:

  • approxEqual is deprecated.
  • isClose has higher precision demands.
  • Because of that you had to up the precision of tested values.
  • But there were too many so you didn't manage all.

It seems to me for the modules you did manage to fix you would want to use the isClose with the sane defaults.

Instead of hijacking approxEqual to call isClose on newer compiler, perhaps it is better to have isClose call approxEqual on older ones.

For the modules you didn't manage to fix you should not hide the deprecation warnings, however horrid they are.

@nordlow
Copy link
Author

nordlow commented Sep 6, 2021

I think I've found a way to automate the precision refactoring of the assert constants...

source/dstats/distrib.d Outdated Show resolved Hide resolved
@John-Colvin
Copy link
Member

Shall I move

static if (__VERSION__ < 2096)
        alias approxEqual = std.math.approxEqual;
    else
        bool approxEqual(T, U, V)(T lhs, U rhs, V maxRelDiff = 1e-2, V maxAbsDiff = 1e-5)
        {
            return std.math.isClose(lhs, rhs, maxRelDiff, maxAbsDiff); // mimic old sloppy approxEqual for now
        }

to a common place? Say, base.d?

yes

@nordlow nordlow changed the title Get rid of deprecation warnings Draft: Get rid of deprecation warnings Sep 13, 2021
@nordlow
Copy link
Author

nordlow commented Sep 13, 2021

Gonna try update the constants now and hopefully get rid of the approxEqual wrapper altogether.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants