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

None Object is returned on .learn_one #2004

Merged
merged 1 commit into from
May 23, 2024
Merged

None Object is returned on .learn_one #2004

merged 1 commit into from
May 23, 2024

Conversation

Proteusiq
Copy link
Contributor

removed the assignment as it would self.model to None. self.model.predict_one returns None. Just like dictionary update. So assignment kill the object.

removed the assignment as it would self.model to None. self.model.predict_one returns None. Just like dictionary update. So assignment kill the object.
@MaartenGr
Copy link
Owner

Ah, that should indeed be changed. No comments here, LGTM!

@MaartenGr MaartenGr merged commit be9376c into MaartenGr:master May 23, 2024
@Proteusiq Proteusiq deleted the patch-1 branch May 24, 2024 15:18
@hamedbh
Copy link

hamedbh commented Jun 13, 2024

I'm not sure if the online docs have been re-rendered since this branch was merged, but the definition of the River class in the river section for Online Topic Modelling still has the incorrect assignment syntax for self.model.learn_one(umap_embedding), i.e.:

from river import stream
from river import cluster

class River:
    def __init__(self, model):
        self.model = model

    def partial_fit(self, umap_embeddings):
        for umap_embedding, _ in stream.iter_array(umap_embeddings):
            self.model = self.model.learn_one(umap_embedding)

        labels = []
        for umap_embedding, _ in stream.iter_array(umap_embeddings):
            label = self.model.predict_one(umap_embedding)
            labels.append(label)

        self.labels_ = labels
        return self

@MaartenGr
Copy link
Owner

@hamedbh Ah right, I forgot to update that documentation. I just did and it should now be correct.

@hamedbh
Copy link

hamedbh commented Jun 14, 2024

@MaartenGr that's great, thank you!

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