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

Solving the bug with max_variance method #128

Merged
merged 3 commits into from
Nov 22, 2019
Merged

Conversation

gifuni
Copy link
Contributor

@gifuni gifuni commented Nov 21, 2019

Converts pandas.Series to DataFrame to resolve merging error in probes.py when using max_variance method.

Solving issue #124

gifuni and others added 3 commits November 21, 2019 16:18
While @gifuni was squashing a bug for some version of pandas wherein
trying to use `pd.merge` on two `pd.Series` objects failed, he
discovered another bug that was plaguing `probe_selection='pc_loading`
where a very strange TypeError was raised. I think this minor change
(returning df.index[0] and not np.squeeze(df.index)) fixes it!

Also, adds an expected blank line
[FIX] Fixes strange TypeError for pc_loading
@codecov
Copy link

codecov bot commented Nov 22, 2019

Codecov Report

Merging #128 into master will increase coverage by 0.12%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #128      +/-   ##
==========================================
+ Coverage   90.36%   90.48%   +0.12%     
==========================================
  Files          32       32              
  Lines        1993     1987       -6     
==========================================
- Hits         1801     1798       -3     
+ Misses        192      189       -3
Impacted Files Coverage Δ
abagen/probes.py 97.05% <100%> (-0.06%) ⬇️
abagen/datasets/fetchers.py 93.05% <0%> (+3.58%) ⬆️

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 7e1db59...00fe908. Read the comment docs.

@rmarkello
Copy link
Owner

This looks great! Thanks so much for the contribution, @gifuni 🎉

@rmarkello rmarkello merged commit de700b5 into rmarkello:master Nov 22, 2019
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.

2 participants