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

Add SSSOM rewire to rewire method #403

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Conversation

matentzn
Copy link
Collaborator

@matentzn matentzn commented Aug 3, 2023

This PR introduces the possibility of rewiring an SSSOM table based on another SSSOM table. This is in scope for sssom-py because of the complex handing of SSSOM mapping set metadata.

@matentzn
Copy link
Collaborator Author

matentzn commented Aug 3, 2023

@ehartley would you be interested to take this on? I created the basic outline of the method (without testing anything of course).

This will solve the sssom- rewiring problem at least. We can think of a generic client later, but note that due to the possibly complex handling of prefix map and metadata it makes sense to have a solid solution here as well.

@ehartley
Copy link

ehartley commented Aug 3, 2023

@matentzn yes, I can take this on.

@matentzn
Copy link
Collaborator Author

matentzn commented Aug 3, 2023

Super!

@cthoyt
Copy link
Member

cthoyt commented Sep 19, 2023

#426 is about fixing the reconciliation code, which also effectively includes the solution to this

@matentzn
Copy link
Collaborator Author

@cthoyt Amazing! Ok then. @ehartley hold off then on any further thinking in this direction, lets wait for Charlie to tell us what, if anything, will be needed on top of his insane work right now :) Thanks @cthoyt

@cthoyt
Copy link
Member

cthoyt commented Sep 27, 2023

@ehartley I added the code that standardizes a MSDF based on recent updates. You can continue on here. I realize I may have misunderstood what was going on here before, so I highly suggest writing several clear, explicit examples as unit tests before starting any implementation.

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.

3 participants