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

FR: SAM (Significance Analysis of Microarrays) #237

Merged
merged 12 commits into from
Jan 25, 2023

Conversation

zieglerSe
Copy link
Contributor

Please reference the issue(s) this PR is related to

Implementing SAM as in FR #236

Please list the changes introduced in this PR

  • Implementation of SAM
  • Documentation
  • Unit Tests

Change of QValues and significant Gene Count, added standard permutation scheme getPermutations
implemented new cut function and monotonized delta
changes in naming (e.g. bioitem replacing gene)
Add unit tests
docs/Testing.fsx Outdated
Comment on lines 999 to 1001
let data1 = Array.map2 tupel rowheader preData1
let data2 = Array.map2 tupel rowheader preData2
Copy link
Member

Choose a reason for hiding this comment

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

replace with

let data1 = Array.zip rowheader preData1
let data2 = Array.zip rowheader preData2

docs/Testing.fsx Show resolved Hide resolved
docs/Testing.fsx Outdated Show resolved Hide resolved
docs/Testing.fsx Outdated
@@ -834,7 +842,7 @@ aErrorAcc
(***hide***)
aErrorAcc |> GenericChart.toChartHTML
(***include-it-raw***)

aErrorAcc|> Chart.show
Copy link
Member

Choose a reason for hiding this comment

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

Remove this Chart.show

docs/Testing.fsx Show resolved Hide resolved
docs/Testing.fsx Show resolved Hide resolved
docs/Testing.fsx Show resolved Hide resolved
@bvenn
Copy link
Member

bvenn commented Nov 27, 2022

ping

@bvenn
Copy link
Member

bvenn commented Dec 6, 2022

Additionally, add a fold change field in the SAM type.

docs/Testing.fsx Outdated
Comment on lines 1044 to 1051
let nonchart =
Chart.Point(nonex,res.NonSigBioitem |> Array.map (fun x -> x.Statistics))
|> Chart.withLineStyle(Color=Color.fromKeyword Gray)
|> Chart.withTraceInfo("no change",Visible = Visible.True)

// negative significant changes
let negex = expected.[0 .. res.NegSigBioitem.Length-1]
Copy link
Member

Choose a reason for hiding this comment

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

let nonex = expected.[res.NegSigBioitem.Length-1 ..
let negex = expected.[0 .. res.NegSigBioitem.Length-1]

Here the (res.NegSigBioitem.Length-1)th element is assigned to both, nonex and negex. I think an index error occurred.

Copy link
Member

@bvenn bvenn Dec 6, 2022

Choose a reason for hiding this comment

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

I think it should be:

let posExpected = expected.[res.NegSigBioitem.Length + res.NonSigBioitem.Length .. res.NegSigBioitem.Length + res.NonSigBioitem.Length + res.PosSigBioitem.Length-1]
let nonex = expected.[res.NegSigBioitem.Length .. res.NegSigBioitem.Length + res.NonSigBioitem.Length-1]
let negex = expected.[0 .. res.NegSigBioitem.Length-1]

Please doublecheck

Comment on lines 594 to 596
let PosSigBioitem = getQvals |> Array.filter (fun x -> x.Statistics > upperCut)
let NegSigBioitem = getQvals |> Array.filter (fun x -> x.Statistics < lowerCut)
let NonSigBioitem = getQvals |> Array.filter (fun x -> x.Statistics >= lowerCut && x.Statistics <= upperCut)
Copy link
Member

Choose a reason for hiding this comment

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

shouldnt it be:

let PosSigBioitem = getQvals |> Array.filter (fun x -> x.Statistics >= upperCut)
let NegSigBioitem = getQvals |> Array.filter (fun x -> x.Statistics <= lowerCut)
let NonSigBioitem = getQvals |> Array.filter (fun x -> x.Statistics > lowerCut && x.Statistics < upperCut)

@codecov-commenter
Copy link

Codecov Report

Merging #237 (0602ff1) into developer (3aa4c4c) will increase coverage by 1.46%.
The diff coverage is 75.90%.

@@              Coverage Diff              @@
##           developer     #237      +/-   ##
=============================================
+ Coverage      43.36%   44.82%   +1.46%     
=============================================
  Files            142      143       +1     
  Lines          14028    14302     +274     
  Branches        1892     1908      +16     
=============================================
+ Hits            6083     6411     +328     
+ Misses          7444     7365      -79     
- Partials         501      526      +25     
Impacted Files Coverage Δ
tests/FSharp.Stats.Tests/Main.fs 0.00% <0.00%> (ø)
src/FSharp.Stats/Testing/SAM.fs 71.68% <71.68%> (+71.68%) ⬆️
tests/FSharp.Stats.Tests/Testing.fs 100.00% <100.00%> (ø)
src/FSharp.Stats/SpecialFunctions/Erf.fs 98.03% <0.00%> (-1.97%) ⬇️
src/FSharp.Stats/Fitting/NonLinearRegression.fs 16.37% <0.00%> (-0.50%) ⬇️
tests/FSharp.Stats.Tests/SpecialFunctions.fs 100.00% <0.00%> (ø)
src/FSharp.Stats/Signal/QQPlot.fs 0.00% <0.00%> (ø)
src/FSharp.Stats/AlgTypes.fs 29.34% <0.00%> (+0.13%) ⬆️
tests/FSharp.Stats.Tests/Distributions.fs 84.46% <0.00%> (+0.42%) ⬆️
... and 5 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@bvenn bvenn merged commit f73b05f into fslaborg:developer Jan 25, 2023
@bvenn bvenn deleted the SAM-Rework branch January 25, 2023 07:21
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.

None yet

3 participants