You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This PR addresses issue #135 by refactoring how the leidenalg dependency is managed.
Details
The leidenalg package is currently listed as a mandatory dependency, but it is not used within the core module. Its only usage occurs in tutorial notebooks located under the docs/ directory. Since it is not essential for the primary functionality of the package, I'd suggest to treat it as an optional dependency.
Changes
Moved leidenalg from the mandatory dependencies to the extras_require section.
Updated relevant documentation to reflect this change.
Impact
This change reduces the dependency footprint for users who do not require the tutorial notebooks, improving installation performance and removing the GPL 2.0 licensed package (i.e. leidenalg) from the dependencies.
@chien-liu, yes, I agree, adding the extra leidenalg dependency to docs/requirements.txt makes sense.
However, leidenalg isn't only used in the tutorials. In classifier.py, the function sc.tl.leiden can also be called, which depends on leidenalg for some scanpy versions. Given this, do you still think it's best to include leidenalg only in docs/requirements.txt, or should we consider listing it as is in the requirements.txt?
In classifier.py, the function sc.tl.leiden can also be called, which depends on leidenalg for some scanpy versions.
In this case, I’d recommend allowing scanpy to manage its own optional dependencies. In other words, we shouldn't explicitly list leidenalg in requirements.txt, since package managers like pip will handle its installation if it's truly required by the specific scanpy version in use.
In addition, looking at the source of sc.tl.leiden (_leiden.py), it appears that leidenalg is only referenced in a type hint. These hints are safely ignored if the module isn’t available, so this shouldn’t cause any issues for users who don’t install leidenalg.
Happy to hear your thought
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR addresses issue #135 by refactoring how the
leidenalgdependency is managed.Details
The leidenalg package is currently listed as a mandatory dependency, but it is not used within the core module. Its only usage occurs in tutorial notebooks located under the
docs/directory. Since it is not essential for the primary functionality of the package, I'd suggest to treat it as an optional dependency.Changes
leidenalgfrom the mandatory dependencies to theextras_requiresection.Impact
This change reduces the dependency footprint for users who do not require the tutorial notebooks, improving installation performance and removing the GPL 2.0 licensed package (i.e.
leidenalg) from the dependencies.