-
-
Notifications
You must be signed in to change notification settings - Fork 247
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
Fit transform #145
base: master
Are you sure you want to change the base?
Fit transform #145
Conversation
This reverts commit c039fbb. Attributed node embeddings Need different fit_transform method that can account for features
stochastic therefore check shape
Apologies, long day and thought I'd opened this PR on my fork to test coverage, CI etc. |
Interesting, all passes locally. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #145 +/- ##
==========================================
+ Coverage 97.41% 97.54% +0.12%
==========================================
Files 63 63
Lines 2707 2849 +142
==========================================
+ Hits 2637 2779 +142
Misses 70 70 ☔ View full report in Codecov by Sentry. |
NB NEU is a meta model!
Add set_params to Estimator base class
I have tried to run the test suite of this pull request, but it is currently failing at the HOPE model test. I see that you are comparing the two embeddings - maybe there are numerical instabilities that lead to different results over different runs? I am not familiar with the internals of numpy & scipy that much. |
Added
.fit_transform
method to all node embedding algorithms, primarily motivated by desire to usekarateclub
algorithms in ascikit-learn
pipeline.Adds:
y=None
argument, for scikit-learn compatibilityif y is not None
to allow passing e.g. node attributes through for a downstream task in the pipelineTests:
.get_embedding()