datatypes.convert to infer indirect (chained) conversions when direct conversions are not implemented#1460
Closed
chrisholder wants to merge 1 commit intomainfrom
Closed
datatypes.convert to infer indirect (chained) conversions when direct conversions are not implemented#1460chrisholder wants to merge 1 commit intomainfrom
chrisholder wants to merge 1 commit intomainfrom
Conversation
Collaborator
|
Really nice, thanks a lot! Much needed. Before discuss this though, let's focus on #1423 and get this merged - that would require hammering out the last areas of design questions and making potential changes there. So I'd recommend to focus on #1423 design write-up and a round (perhaps targeted meetings) with @ABostrom (because of the link to the proposed typing in #974, #973). |
Collaborator
|
@chrisholder, could we keep these open? I might be using this someday! |
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reference Issues/PRs
Requires PR #1423
What does this implement/fix? Explain your changes.
The current convert module only allows direct conversions. For example currently if I have a nested_univ and want to make it into a numpy_flat I am unable to do this. This is because there is no direct conversion between the two implemented. However, there is an indirect conversion that can be done which is nested_univ -> numpy3D -> numpy_flat. This pr has updated the convert module to allow these indirect paths to be found and used. Therefore this pr greatly improves the coverage of conversions.
It does this by using an adjacency matrix which defines which conversion are avaliable and then using dijsktras optimal path to find an indirect path.
In addition using this methods the weights (or length of the paths) can be adjusted to only allow lossy or lossless conversions. This is taken into account and when the user wants a lossless conversion to be done paths that would lead to loss are not considered. However, when the user is fine with loss then all paths are considered.
Does your contribution introduce a new dependency? If yes, which one?
What should a reviewer concentrate their feedback on?
Requires PR #1423 as this contains the enums needed to find paths.
Any other comments?
PR checklist
For all contributions
For new estimators