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

Expose random_state in leiden #102

Merged
merged 8 commits into from Dec 13, 2023
Merged

Expose random_state in leiden #102

merged 8 commits into from Dec 13, 2023

Conversation

jpintar
Copy link
Contributor

@jpintar jpintar commented Dec 3, 2023

feat(leiden): expose additional parameter

  1. expose random_state parameter to better align with Scanpy
  2. pass it to cuGraph via culeiden
  3. save its value in .uns["leiden"]["params"]
  4. describe it in docstring
  5. make multiple small improvements to docstring

N.b., the theta parameter was not exposed because a bug in cuGraph (present at commit 1e446c4) makes it have no effect; this should be re-visited when cuGraph is updated.

Resolves first half of #101

jpintar and others added 3 commits December 2, 2023 19:36
feat(leiden): expose additional parameter

1) expose 'random_state' parameter to better align with Scanpy
2) pass it to cuGraph via 'culeiden'
3) save its value in '.uns["leiden"]["params"]'
4) describe it in docstring
5) make multiple small improvements to docstring

N.b., the 'theta' parameter was not exposed because a bug in cuGraph (present at commit 1e446c4) makes it have no effect; this should be re-visited when cuGraph is updated.
fix(leiden): fix undefined type

Import 'Union' from 'typing' to fix error when defining 'random_state' (line 17)
@Intron7 Intron7 added the run-gpu-ci runs GPU CI label Dec 3, 2023
@Intron7 Intron7 self-requested a review December 12, 2023 19:31
Copy link
Member

@Intron7 Intron7 left a comment

Choose a reason for hiding this comment

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

Please also add a release note.

Intron7 and others added 3 commits December 12, 2023 22:54
Release note describing addition of `random_state` parameter to `leiden`
Include 0.9.4.md
@jpintar jpintar requested a review from Intron7 December 13, 2023 00:04
Copy link
Member

@Intron7 Intron7 left a comment

Choose a reason for hiding this comment

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

Looks good to me

@Intron7 Intron7 merged commit 32c940c into scverse:main Dec 13, 2023
5 checks passed
@jpintar jpintar deleted the leiden-fix branch December 13, 2023 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-gpu-ci runs GPU CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants