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

PCA Diamond problem fix #658

Merged
merged 3 commits into from Dec 9, 2015

Conversation

Projects
None yet
2 participants
@patricksnape
Contributor

patricksnape commented Dec 9, 2015

This was introduced by me - I take full responsibility. Previously, the inheritance chain meant that PCAModel was picking up the project method of PCAVectorModel - and therefore failing when passed a Vectorizable object. Merely swapping the inheritance fixes this.

Also, rename MeanLinearModel and LinearModel to state that they actually operate on numpy arrays (with the new PCAVectorModel naming scheme). Keep the old classes around as aliases for backwards compatibility.

patricksnape added some commits Dec 9, 2015

HOTFIX: Diamond problem
Very embarassing - introduced a diamond problem on PCAModel,
we need the vectorizable implementation - so that class
must be inherited from first.
Rename LinearModel and MeanLinearModel for consistency
Keep an alias to stop a breaking change but they should have
vector in the name given the change to PCAModel (PCAVectorModel).
The old names will be deprecated in 0.7.0.
Add test that fails on master but passes here
Test that project works! This proves that the diamond problem
is no more
@jabooth

This comment has been minimized.

Member

jabooth commented Dec 9, 2015

Looks good, +1 once App Veyor passes.

jabooth added a commit that referenced this pull request Dec 9, 2015

Merge pull request #658 from patricksnape/pca_fix
PCA Diamond problem fix

@jabooth jabooth merged commit cf06852 into menpo:master Dec 9, 2015

4 checks passed

OS X MenpoBot Jenkins build passed No test results found.
Details
clahub All contributors have signed the Contributor License Agreement.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jabooth jabooth removed the in progress label Dec 9, 2015

@jabooth jabooth deleted the patricksnape:pca_fix branch Dec 9, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment