Skip to content

chore: add speed note to leiden warning#3949

Merged
flying-sheep merged 8 commits into
mainfrom
ig/speed_warning
Jan 23, 2026
Merged

chore: add speed note to leiden warning#3949
flying-sheep merged 8 commits into
mainfrom
ig/speed_warning

Conversation

@ilan-gold
Copy link
Copy Markdown
Contributor

@ilan-gold ilan-gold commented Jan 21, 2026

@ilan-gold ilan-gold added this to the 1.12.0 milestone Jan 21, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 21, 2026

Codecov Report

❌ Patch coverage is 85.18519% with 4 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@9aaf126). Learn more about missing BASE report.
⚠️ Report is 52 commits behind head on main.

Files with missing lines Patch % Lines
src/scanpy/tools/_leiden.py 85.18% 4 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3949   +/-   ##
=======================================
  Coverage        ?   78.26%           
=======================================
  Files           ?      117           
  Lines           ?    12633           
  Branches        ?        0           
=======================================
  Hits            ?     9887           
  Misses          ?     2746           
  Partials        ?        0           
Flag Coverage Δ
hatch-test.low-vers 77.59% <85.18%> (?)
hatch-test.pre 77.22% <85.18%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/scanpy/tools/_leiden.py 93.50% <85.18%> (ø)

Comment thread src/scanpy/tools/_leiden.py Outdated
ilan-gold and others added 2 commits January 22, 2026 13:29
Co-authored-by: Lukas Heumos <lukas.heumos@posteo.net>
Copy link
Copy Markdown
Member

@flying-sheep flying-sheep left a comment

Choose a reason for hiding this comment

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

Hmm, two warnings when flavor=None … I think it’s OK, but maybe it would be more elegant to just add the text to the FutureWarning when raising that one, and only raise the UserWarning when specifying flavor="leidenalg" manually?

Comment thread pyproject.toml
Comment on lines +199 to +200
# igraph vs leidenalg warning
"ignore:The `igraph` implementation of leiden clustering:UserWarning",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why not do this more locally?

@ilan-gold
Copy link
Copy Markdown
Contributor Author

Hmm, two warnings when flavor=None … I think it’s OK, but maybe it would be more elegant to just add the text to the FutureWarning when raising that one, and only raise the UserWarning when specifying flavor="leidenalg" manually?

But I think we want people definitely using igraph either way - if you don't specify anything and if you do, either way you should be using igraph.

@flying-sheep
Copy link
Copy Markdown
Member

flying-sheep commented Jan 22, 2026

That’s why I suggested

maybe it would be more elegant to just add the [new] text to the FutureWarning when raising that one [and raising the new text as a standalone UserWarning otherwise]

that way, the user would get

  • max 1 warning
  • warning text that talks about future behehavior when relying on default behavior
  • warning text that talks about performance when not using igraph

@flying-sheep flying-sheep enabled auto-merge (squash) January 23, 2026 09:16
@flying-sheep flying-sheep merged commit f70e681 into main Jan 23, 2026
14 checks passed
@flying-sheep flying-sheep deleted the ig/speed_warning branch January 23, 2026 10:02
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.

leiden: warn about speed of igraph flavor being **much** faster than leidenalg flavor

3 participants