Skip to content
This repository has been archived by the owner on Mar 1, 2024. It is now read-only.

Update calls to round, change signif to round #56

Merged
merged 4 commits into from
Apr 10, 2018

Conversation

ScottPJones
Copy link
Collaborator

No description provided.

@ScottPJones ScottPJones changed the title Spj/round Update calls to round, change signif to round Apr 10, 2018
@ScottPJones
Copy link
Collaborator Author

The failure on nightly is caused by another bug introduced by the same change JuliaLang/julia#26670, that will need to be fixed in base.

@jmkuhn jmkuhn merged commit 531efe6 into JuliaIO:master Apr 10, 2018
@ScottPJones
Copy link
Collaborator Author

Thanks for merging this!

@ScottPJones ScottPJones deleted the spj/round branch April 10, 2018 13:57
# BigFloat and DecFP (at least) on master

@static if VERSION >= v"0.7.0-DEV.4804"
# adapted from Matlab File Exchange roundsd: http://www.mathworks.com/matlabcentral/fileexchange/26212
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not use the Matlab file exchange. While the original author is required to license their work as BSD, by downloading it from the file exchange you're agreeing to this additional clause in their terms of use:

Content submitted to File Exchange may only be used with MathWorks products.

Besides that, a URL in source code does not satisfy the terms of the BSD license for the original author — that copyright notice should be added or linked to in the LICENSE.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was taken directly from Julia base/floatfunctions.jl (in v0.6.x).
If there is a problem with Julia base, then it should be corrected there as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, indeed. Thanks!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As soon as it is fixed in base (and hopefully supported by Compat.jl), this can be changed to simply call the new form, i.e. round(ax; sigdig=fs.prec + 1).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, glad to see it's being removed. The biggest thing is actually the ToU violation for the person that actually downloaded it, but even then that clause isn't very clear. I just try to stay away from the file exchange, personally.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep - good advice (to stay away from MathWorks).
I hadn't thought about it, since I'd assumed that things in Base had been well vetted.
(and you know what happens when you assume anything!)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found the original change - and it's an ancient one! 5.5 years old, from Sept. 4, 2012 (JuliaLang/julia@4c88c22) by HarlanH, committed by Stefan.
Not sure if the violation is considered to be by the person who made the PR, or the person who actually placed it into the Julia source code against the ToU.

simonbyrne added a commit to JuliaLang/julia that referenced this pull request Apr 10, 2018
Fixes BigFloat digit rounding (JuliaIO/Formatting.jl#56). I've also tweaked the definitions to make it easier to extend to new number formats.
simonbyrne added a commit to JuliaLang/julia that referenced this pull request Apr 12, 2018
Fixes BigFloat digit rounding (JuliaIO/Formatting.jl#56). I've also tweaked the definitions to make it easier to extend to new number formats.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants