Skip to content

implement rand() - #61

Merged
timholy merged 1 commit into
JuliaMath:masterfrom
bjarthur:bja/rand
Nov 16, 2016
Merged

timholy merged 1 commit into
JuliaMath:masterfrom
bjarthur:bja/rand

Conversation

@bjarthur

@bjarthur bjarthur commented Nov 6, 2016

Copy link
Copy Markdown
Collaborator

this should set the stage for a subsequent effort to make rand(Gray{UFixed{UInt16,14}}, 3) work.

@codecov-io

codecov-io commented Nov 6, 2016

Copy link
Copy Markdown

Current coverage is 88.62% (diff: 100%)

Merging #61 into master will increase coverage by 0.20%

@@             master        #61   diff @@
==========================================
  Files             3          3          
  Lines           164        167     +3   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            145        148     +3   
  Misses           19         19          
  Partials          0          0          

Powered by Codecov. Last update ccb4267...ed63302

@timholy

timholy commented Nov 6, 2016

Copy link
Copy Markdown
Member

LGTM, but notice the test failures.

@bjarthur
bjarthur force-pushed the bja/rand branch 4 times, most recently from dd29f88 to a2647ea Compare November 12, 2016 21:14
@bjarthur

bjarthur commented Nov 12, 2016

Copy link
Copy Markdown
Collaborator Author

i've simplified the tests in this most recent force push.

also, please tell me what you think of the hack around JuliaLang/julia#10140

@bjarthur

Copy link
Copy Markdown
Collaborator Author

i removed the hack, and benchmarked. much faster than the original PR now. i think it's ready to go. will wait a few days for comments before merging.

@timholy timholy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@timholy
timholy merged commit 8eb2fb6 into JuliaMath:master Nov 16, 2016
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