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

Fixed need for intercept #62

Merged
merged 4 commits into from
Jul 18, 2022
Merged

Fixed need for intercept #62

merged 4 commits into from
Jul 18, 2022

Conversation

wfondrie
Copy link
Owner

This PR resolves Issue #61 and does a little bit of cleaning in the mokapot.brew() function.

The CV fold that mokapot.Model() objects are fit to is now recorded during the fitting process. This means we can successfully recover the order of trained models using the recorded folds.

@wfondrie wfondrie requested a review from surya-money July 16, 2022 07:29
@codecov-commenter
Copy link

codecov-commenter commented Jul 16, 2022

Codecov Report

Merging #62 (74251d5) into master (bca014d) will increase coverage by 0.92%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #62      +/-   ##
==========================================
+ Coverage   83.24%   84.16%   +0.92%     
==========================================
  Files          18       18              
  Lines        1552     1554       +2     
==========================================
+ Hits         1292     1308      +16     
+ Misses        260      246      -14     
Impacted Files Coverage Δ
mokapot/config.py 91.30% <ø> (ø)
mokapot/brew.py 90.47% <100.00%> (+1.05%) ⬆️
mokapot/model.py 86.93% <100.00%> (+3.09%) ⬆️
mokapot/mokapot.py 93.90% <0.00%> (+8.53%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bca014d...74251d5. Read the comment docs.

# Only way I found to sort is using intercept values
models.sort(key=lambda x: x[0].estimator.intercept_)
fitted.sort(key=lambda x: x[0].fold)
models, resets = list(zip(*fitted))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Little bit confused by what this line does

Copy link
Owner Author

Choose a reason for hiding this comment

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

It's a little trick to turn a list of lists/tuples into two lists:

x = [("a", 1), ("b", 2), ("c", 3)]
y, z = list(zip(*x))
print(y)
# ["a", "b", "c"]
print(z)
# [1, 2, 3]

mokapot/brew.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@surya-money surya-money left a comment

Choose a reason for hiding this comment

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

Changes look good! I just had a question about the purpose regarding one line of code and a super minor suggestion (all in brew.py) but otherwise everything makes sense!

@wfondrie wfondrie merged commit 21680cc into master Jul 18, 2022
@wfondrie wfondrie deleted the issue61 branch July 18, 2022 17:03
sambenfredj referenced this pull request in msaid-de/mokapot Apr 12, 2023
…t' into 'develop'

refactor implementation of psms streaming object

Closes #62

See merge request msaid/chimerys/mokapot!51
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