Opened 9 years ago

Closed 8 years ago

#9080 closed enhancement (fixed)

add F-distribution support for RealDistribution

Reported by: klee Owned by: amhou
Priority: minor Milestone: sage-4.7.2
Component: statistics Keywords:
Cc: Merged in: sage-4.7.2.alpha0
Authors: Kwankyu Lee Reviewers: Karl-Dieter Crisman
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

add the F-distribution to the supported distributions for RealDistribution

Attachments (3)

trac#9080.patch​ (10.6 KB) - added by klee 9 years ago.
trac_9080.patch​ (10.7 KB) - added by klee 9 years ago.
revised according to the reviewer's comments
trac_9080-rebased.patch​ (11.0 KB) - added by kcrisman 8 years ago.

Download all attachments as: .zip

Change History (12)

Changed 9 years ago by klee

comment:1 Changed 9 years ago by klee

  • Status changed from new to needs_review

The patch adds F-distribution.

comment:2 Changed 9 years ago by kcrisman

  • Status changed from needs_review to needs_work

You may want to rebase this against 4.6.alpha1 if necessary. Also, the commit message should start with Trac 9080 or something like that. Finally, maybe the error message should say that this is not a supported distribution? It's certainly conceivable that there would be one in the literature which isn't yet in Sage or GSL :)

comment:3 Changed 9 years ago by klee

  • Status changed from needs_work to needs_review

I rebased the patch to the latest release of Sage (4.5.3). It was fun (and sad...) to see all different flavors of the heads of the commit messages used by developers. I followed the one DEMANDED by the developer manual. I changed all the error message as suggested.

Changed 9 years ago by klee

revised according to the reviewer's comments

comment:4 follow-up: Changed 9 years ago by klee

There is a slight inconsistency even in the developer manual. In the section "Producing Patches with Mercurial", "trac xxxx: ..." is suggested while "Trac xxxx: ..." seems to be the official standard.

comment:5 in reply to: ↑ 4 Changed 9 years ago by kcrisman

Replying to klee:

There is a slight inconsistency even in the developer manual. In the section "Producing Patches with Mercurial", "trac xxxx: ..." is suggested while "Trac xxxx: ..." seems to be the official standard.

You could ask on sage-devel about this - for me, it's not so crucial, but mvngu might have an informed opinion.

Changed 8 years ago by kcrisman

comment:6 Changed 8 years ago by kcrisman

I've rebased this against 4.7.1.alpha2.

comment:7 Changed 8 years ago by kcrisman

  • Reviewers set to Karl-Dieter Crisman
  • Status changed from needs_review to positive_review

Positive review. Gives correct values, checked with Wolfram Alpha. Not surprising, since GSL is a very stable library.

But we need a lot more doctests for just about all of these - see #11514.

comment:8 Changed 8 years ago by jdemeyer

  • Milestone changed from sage-4.7.1 to sage-4.7.2

comment:9 Changed 8 years ago by jdemeyer

  • Merged in set to sage-4.7.2.alpha0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.