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

adding the ability to output the dataset #1502

Closed
wants to merge 1 commit into from

Conversation

jkobject
Copy link

Hello Sergey, Alex,

I just realized I had made one more update to .mapped() in which I am adding the ability to output the dataset. I need it to remove batch effects in my model like scGPT does.

Let me know if that is ok. Once this is updated I can use this version :)

adding the ability to output the dataset (this is useful for removing batch effects)
@falexwolf
Copy link
Member

Thanks @jkobject! @Koncopd could you take a look at this so that it gets into the new release?

@@ -79,6 +79,7 @@ def __init__(
unknown_label: Optional[Union[str, Dict[str, str]]] = None,
cache_categories: bool = True,
parallel: bool = False,
add_dataset_id: bool = False,
Copy link
Member

Choose a reason for hiding this comment

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

@Koncopd - I don't fully understand what this does, but likely it deserves a better name.

@Koncopd
Copy link
Member

Koncopd commented Mar 15, 2024

Hm, maybe return this by default without any additional arguments? As we return a dict now, it is not a problem if we add some additinal keys.

@falexwolf
Copy link
Member

Sounds like the right thing, Sergei!

@Koncopd
Copy link
Member

Koncopd commented Mar 17, 2024

@jkobject are you fine with the suggestion? I can do this myself or you can change it here - just by removing the argument add_dataset_id. And lets call the key "_storage_idx" in out.

@jkobject
Copy link
Author

Works with me!

@Koncopd
Copy link
Member

Koncopd commented Mar 17, 2024

done here #1504

@Koncopd Koncopd closed this Mar 17, 2024
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