Skip to content

Conversation

@leerosenthalj
Copy link
Contributor

Initial tests didn't break anything but some compatibility details in the Project.toml might need to be changed

@giordano
Copy link
Member

giordano commented Jun 9, 2025

Tests seem to be failing here.

@leerosenthalj
Copy link
Contributor Author

Thanks for checking. Would it be possible for someone more familiar with this project to take a look at the changes? The tests are failing for me too, and I am not sure where the issue is.

@giordano
Copy link
Member

giordano commented Jun 9, 2025

Tests are passing on main though

@leerosenthalj
Copy link
Contributor Author

leerosenthalj commented Jun 9, 2025

Yup, the issue is definitely in my changes. The ItemGraphs compatibility is only an issue in one file, if someone else is willing to take a look that would be great

@leerosenthalj
Copy link
Contributor Author

It's entirely in TimeScales.jl

@helgee
Copy link
Member

helgee commented Jun 11, 2025

I pushed a patch that fixes the tests. The problem was that add_vertex! used to return the vertex index in LightGraphs.jl but just returns true in Graphs.jl if the insert was successful.

Sorry for the long delay @leerosenthalj. I have been completely under water for the better part of two years now...

helgee and others added 2 commits June 11, 2025 20:18
Co-authored-by: Mosè Giordano <[email protected]>
Co-authored-by: Mosè Giordano <[email protected]>
Co-authored-by: Mosè Giordano <[email protected]>
@giordano
Copy link
Member

Yay, all green now, thanks everybody!

@helgee helgee merged commit 3a9b621 into JuliaAstro:main Jun 11, 2025
18 checks passed
@giordano giordano linked an issue Jun 11, 2025 that may be closed by this pull request
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.

Switch from LightGraphs to Graphs for compatibility

4 participants