Skip to content
This repository was archived by the owner on Jul 28, 2025. It is now read-only.

Recursion error in to_matrix#111

Open
Neclow wants to merge 10 commits into
uw-ssec:devfrom
Neclow:matrix
Open

Recursion error in to_matrix#111
Neclow wants to merge 10 commits into
uw-ssec:devfrom
Neclow:matrix

Conversation

@Neclow

@Neclow Neclow commented Apr 5, 2025

Copy link
Copy Markdown

Fix recursion error in to_matrix, like to_vector (see previous PR)

(Sorry for the short message, on the boat on a Saturday!)

I can add some rust code for that fix later

@deepsource-io

deepsource-io Bot commented Apr 5, 2025

Copy link
Copy Markdown

Here's the code health analysis summary for commits cd49b7d..7254bef. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource Rust LogoRust✅ Success
🎯 3 occurences resolved
View Check ↗
DeepSource Python LogoPython✅ SuccessView Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

@Neclow Neclow changed the title Matrix Recursion error in matrix Apr 5, 2025
@Neclow Neclow changed the title Recursion error in matrix Recursion error in to_matrix Apr 5, 2025
@Neclow Neclow marked this pull request as draft April 5, 2025 19:51
@Neclow Neclow marked this pull request as ready for review April 5, 2025 20:22
@Neclow

Neclow commented Apr 5, 2025

Copy link
Copy Markdown
Author

Added some rust code that somewhat repeats the python code I wrote, I'm not 100% sure how you usually operate with the different versions so I went ahead with both versions. Made a few design choices with the rust code, but happy to discuss/change

@sonarqubecloud

sonarqubecloud Bot commented Apr 6, 2025

Copy link
Copy Markdown

@Neclow

Neclow commented Apr 6, 2025

Copy link
Copy Markdown
Author

Also fixed all current tests, which also pertain to #110. I suggest we can take some time Tuesday to explain the commits, or feel free to point out parts of the PR that should be better explained.

Another TODO would be to have more random tests like I wrote in the original package? e.g., for to_matrix, 1) sample a random matrix, 2) convert to newick, 3) convert back to matrix, 4) check that first matrix == second matrix

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant