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

0.31.6.0: Adds implicit and explicit conversions from double[][] to F64Matrix #124

Merged
merged 4 commits into from
Oct 12, 2019

Conversation

jameschch
Copy link
Contributor

This is a simple way to address the need to expose an API surface that accepts double[][] rather than an F64Matrix. Instead of changing all interfaces and implementations, I have made double[][] implicitly convertible to F64Matrix. This can be considered a convenience and a temporary workaround for #20 and #115 .

@mdabros mdabros self-requested a review October 3, 2019 15:14
@mdabros mdabros self-assigned this Oct 3, 2019
@mdabros
Copy link
Owner

mdabros commented Oct 3, 2019

@jameschch I think this is a good idea. Even though it is a bit memory heavy to convert between double[][] and F64Matrix in this way. But overall, it makes life easier for developers that work with manageable data sizes, and usually use the jagged array type.

Maybe you could add similar methods for supporting multidim arrays double[,]? It can be in a separate/later pull request if that suits you better.

Copy link
Owner

@mdabros mdabros left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jameschch Additions looks good. Just need to fix a few small nit picks :-).
Thanks for contributing with this!

@jameschch
Copy link
Contributor Author

Build seems to be failing after I added some line breaks.

@mdabros
Copy link
Owner

mdabros commented Oct 8, 2019

@jameschch Failing tests seems to be unrelated to your changes, so the failing build is most likely caused by some unstable unit tests. I will have a look at it later this week.

@mdabros
Copy link
Owner

mdabros commented Oct 10, 2019

@jameschch Since the checks fails because of flaky unit tests, I think we should just merge this, and I can look into the flaky tests in another PR.

If you increment the version from 0.31.5.0 to 0.31.6.0 I can merge this. You might need to merge with the master first since #88 was completed in the meantime.

@mdabros mdabros changed the title Adds implicit and explicit conversions from double[][] to F64Matrix 0.31.6.0: Adds implicit and explicit conversions from double[][] to F64Matrix Oct 12, 2019
@mdabros mdabros merged commit 1fe2ac2 into mdabros:master Oct 12, 2019
@mdabros
Copy link
Owner

mdabros commented Oct 12, 2019

@jameschch I have completed the pull request. Thanks for the contribution! packages should be up shortly.

best regards
Mads

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