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

Adding edge encoding utility #14

Closed
wants to merge 3 commits into from
Closed

Conversation

jjustison
Copy link
Member

If wanted, here is a utility function for remapping the species edges onto the gene edges after reading in the gene tree.

Some of the stuff I've been working on uses the edge mappings and these mappings are lost when reading/writing the gene trees.

Let me know if this seems useful but you want more tests, documentation, clarification, etc.

@jjustison jjustison requested a review from cecileane October 31, 2024 18:23
@jjustison jjustison self-assigned this Oct 31, 2024
Copy link

codecov bot commented Oct 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ
src/utils.jl 100.00% <100.00%> (ø)

🚨 Try these New Features:

@cecileane
Copy link
Member

That looks like a very useful and even necessary utility indeed. Thank you so much! I will review asap.
My first high-level thoughts are:

  1. The test would ideally use a gene tree read from file, not a gene tree simulated during the tests, to show the main reason why this function exists and also to have an input gene tree with the default (empty) internal attributes, especially those modified by the function.
  2. Given how useful this function could be to others, it would be great to export it and to add a section or subsection or page showing it in the documentation manual.

@cecileane
Copy link
Member

@jjustison : could you please place this pull request on a branch other than your main branch? that will make it possible for all developers here to make edits to your PR. No rush, as you know!

@jjustison
Copy link
Member Author

I created a new branch called edge_encoding. I am not sure if the the proper git flow but I will close this PR and open one from that branch.

@jjustison jjustison closed this Nov 22, 2024
@jjustison jjustison mentioned this pull request Nov 22, 2024
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.

2 participants