Skip to content
This repository has been archived by the owner on Aug 24, 2020. It is now read-only.

factorial from math #13

Closed
wants to merge 2 commits into from
Closed

factorial from math #13

wants to merge 2 commits into from

Conversation

zsrinivas
Copy link

importing factorial from math

@sublee
Copy link
Contributor

sublee commented Sep 22, 2014

Thanks to patch but the test failed.

Anyway, the factorial function doesn't matter in the test code. It's just a simplest example to test profiling results clearly. We can replace it with any other example.

@zsrinivas
Copy link
Author

👍

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) when pulling fcb8dd0 on eightnoteight:master into 810da9b on what-studio:master.

@zsrinivas zsrinivas closed this Sep 23, 2014
@zsrinivas zsrinivas reopened this Sep 23, 2014
@zsrinivas
Copy link
Author

I think the tests has to be changed for this one..

@zsrinivas zsrinivas closed this Sep 23, 2014
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) when pulling fcb8dd0 on eightnoteight:master into 810da9b on what-studio:master.

@sublee
Copy link
Contributor

sublee commented Sep 23, 2014

Thanks to patch and comment again.

But I still couldn't understand why the test has to use math.factorial. Shall you explain the reason?

@zsrinivas
Copy link
Author

I'm terribly sorry for wasting your time on this,
after your yesterday's comment i wanted to close this and about my comment,
simply because it is fast and written in C and when you are in python3 it is even faster because of algo implementation see this.
BUT all this doesn't matter for the practical purpose of this program.
Sorry again!
I have already tried to close this.(in a very fuzzy way)

@sublee
Copy link
Contributor

sublee commented Sep 23, 2014

Oh, don't sorry for this. I thank for your cooperation. If the factorial's speed be a problem at the future, I'll remind your advice. Thanks again.

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