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

Copied tests and some definitions from base #13

Merged
merged 8 commits into from
Aug 8, 2016

Conversation

ararslan
Copy link
Member

Let's see what happens to the coverage and CI...

@ararslan
Copy link
Member Author

...the answer appears to be "not good things."

In this repo we're doing some of the bessel return values with oftype(x, ...), which is omitted in Base. Should we just copy what Base is doing or is there a good reason to be doing it this way? cc @tkelman

@tkelman
Copy link
Contributor

tkelman commented Jun 28, 2016

I'm not sure! What is the type difference if you leave out the oftype? Can you see anything in the history of the base files where maybe it was changed at some point that this repo missed?

@ararslan
Copy link
Member Author

I can check the return type differences a little later but in the meantime I was able to find the commit that removed the oftypes: JuliaLang/julia@8077d11

@simonbyrne As the one who introduced the change, can you provide some insight into this?

@simonbyrne
Copy link
Member

As the commit comment mentions, it was to fix JuliaLang/julia#11512

@ararslan
Copy link
Member Author

ararslan commented Jun 28, 2016

Right, that fixed the reported issue with BigFloats, I was just wondering if there was any other justification for that particular solution since removing oftype seems like it would make the return type less predictable in general. (Maybe that's totally wrong, I don't know.)

@ararslan
Copy link
Member Author

In particular, it looks like one of the test failures was because oftype was converting the return type, which would otherwise have been a float, to complex to match the argument's type. No idea whether that's desirable.

@ararslan
Copy link
Member Author

ararslan commented Aug 8, 2016

First of all, sorry for the month and a half delay in getting this done.

I think I had misinterpreted the errors before; I don't think Simon's removal of oftype was actually causing the type difference I was seeing. It appears it was a thing with the 0.4 version of isapprox that Simon has since fixed in base. Sorry for the confusion, and thanks to Simon for fixing isapprox!

I worked around the isapprox difference in here and got all AMOS-based code and tests copied over from base.

@ararslan ararslan merged commit 5c0f86f into JuliaMath:master Aug 8, 2016
@ararslan ararslan deleted the aa/base_special branch August 8, 2016 06:31
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