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

Update dependencies, Julia compat, and add weak dependencies #58

Merged
merged 4 commits into from
Oct 6, 2023

Conversation

devmotion
Copy link

This PR updates the list of dependencies: SparseArrays is not needed anymore, it seems #40 removed the use of SparseMatrixCSC in src/stress.jl This cleanup is very helpful since in Julia >= 1.10 will not be included anymore in the default system image and therefore there is an effort throughout the ecosystem to make SparseArrays a weak dependency whenever possible. Removing it from NetworkLayout means that it won't trigger the compilation and loading of these extensions.

I used the opportunity and additionally moved the support for LightGraphs and Graphs in Julia >= 1.9 to extensions. Moreover, when testing the PR I noticed that the package, or rather its dependencies, are not compatible with Julia 1.0 anymore (even though according to the Project.toml file Julia >= 1.0 is supported). Since the CI runs only with Julia >= 1.6 I bumped the Julia compat entry to 1.6.

@codecov
Copy link

codecov bot commented Oct 5, 2023

Codecov Report

Merging #58 (a68aa5b) into master (f16fa63) will decrease coverage by 0.87%.
The diff coverage is 57.14%.

❗ Current head a68aa5b differs from pull request most recent head 8fc54cf. Consider uploading reports for the commit 8fc54cf to get more accurate results

@@            Coverage Diff             @@
##           master      #58      +/-   ##
==========================================
- Coverage   98.32%   97.45%   -0.87%     
==========================================
  Files           8        9       +1     
  Lines         538      551      +13     
==========================================
+ Hits          529      537       +8     
- Misses          9       14       +5     
Files Coverage Δ
src/stress.jl 100.00% <ø> (ø)
src/NetworkLayout.jl 92.20% <66.66%> (-3.39%) ⬇️
ext/NetworkLayoutGraphsExt.jl 50.00% <50.00%> (ø)

@devmotion
Copy link
Author

The docs failure seems unrelated and caused by the breaking changes in Documenter 1 (some kwargs were removed).

@simonschoelly
Copy link
Contributor

This package is not really maintained much by the people that maintain Graphs.jl - I think its mostly used by GraphMakie.jl, so I cannot comment much here - but maybe two questions:

  1. Is there a good reason to keep the support for LightGraphs.jl? The only dependencies of this package that still have LightGraphs as a dependency are PlasmoPlots.jl and Nodariety.jl - and I am not sure if these are even still developed.

  2. Are weak dependencies already supported by Julia 1.6?

@devmotion
Copy link
Author

  1. Is there a good reason to keep the support for LightGraphs.jl?

I was wondering the same but since I'm not a regular contributor to this package I thought it would be best to not perform any major changes, in particular since it did not cause any dependency incompatibilities.

  1. Are weak dependencies already supported by Julia 1.6?

No, weak dependencies are only supported by Julia >= 1.9. On Julia < 1.9, the PR does not change the current behaviour and the additional support for Graphs and LightGraphs will be loaded optionally with Requires.

@hexaeder
Copy link
Collaborator

hexaeder commented Oct 6, 2023

I think it is totally fine to drop LightGraphs support, it was only important during the transition. I also changed the doc build for Documenter@1.0.

Thanks for the PR anyway! The move to PkgExtension was overdue.

PS: Noticed to late that you have a better PR for the Documenter thing already...

@hexaeder hexaeder merged commit 130b7e5 into JuliaGraphs:master Oct 6, 2023
2 of 3 checks passed
@devmotion devmotion deleted the dw/deps branch October 6, 2023 12:08
@cormullion
Copy link

its mostly used by GraphMakie.jl

plus BioLab.jl, EvoTrees.jl, GraphRecipes.jl, ITensorMakie.jl, ITensorUnicodePlots.jl, ITensorVisualizationBase.jl, Karnak.jl, MINDFulMakie.jl, NestedGraphMakie.jl, Nodariety.jl, OptimizedEinsum.jl, PlantGraphs.jl, PlasmoPlots.jl, PowerPlots.jl, PowerSystemsMaps.jl, QUBOTools.jl, QuantumSavory.jl, TreeRecipe.jl ... :)

@simonschoelly
Copy link
Contributor

its mostly used by GraphMakie.jl

plus BioLab.jl, EvoTrees.jl, GraphRecipes.jl, ITensorMakie.jl, ITensorUnicodePlots.jl, ITensorVisualizationBase.jl, Karnak.jl, MINDFulMakie.jl, NestedGraphMakie.jl, Nodariety.jl, OptimizedEinsum.jl, PlantGraphs.jl, PlasmoPlots.jl, PowerPlots.jl, PowerSystemsMaps.jl, QUBOTools.jl, QuantumSavory.jl, TreeRecipe.jl ... :)

You are right, I should have mentioned that. I did check the packages that have NetworkLayout.jl as a dependency though (not recusively, maybe I should have) and apart from the two that I have mentioned, they don't use LightGraphs.jl

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.

4 participants