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

datavec-python #7188

Merged
merged 71 commits into from Apr 1, 2019

Conversation

Projects
None yet
4 participants
@farizrahman4u
Copy link
Member

commented Feb 18, 2019

No description provided.

farizrahman4u added some commits Feb 18, 2019

@farizrahman4u farizrahman4u referenced this pull request Feb 18, 2019

Closed

Add datavec-python #7165

@farizrahman4u farizrahman4u requested review from agibsonccc and saudet Feb 20, 2019

@agibsonccc
Copy link
Member

left a comment

Minor fixes see above for concerns for other data types as well

@farizrahman4u farizrahman4u removed the request for review from saudet Feb 20, 2019

farizrahman4u added some commits Feb 20, 2019

Show resolved Hide resolved pom.xml Outdated

farizrahman4u added some commits Feb 23, 2019

Show resolved Hide resolved pom.xml Outdated
@agibsonccc
Copy link
Member

left a comment

Python version nit

@maxpumperla
Copy link
Contributor

left a comment

I really like this approach of specifying input and output schemas and let the user freely decide what happens in the transform step.

Having said that, I'd really like to see better test coverage (one example below) and basic doc strings for the core abstractions and how to use them. Shouldn't be too much work. Thanks.

farizrahman4u added some commits Feb 27, 2019

farizrahman4u and others added some commits Mar 15, 2019

Update version of JavaCPP, JavaCV, and all presets to 1.5-SNAPSHOT
Including change-cuda-versions.sh to support CUDA 10.1 and cuDNN 7.5
@farizrahman4u

This comment has been minimized.

Copy link
Member Author

commented Mar 19, 2019

Seems we need javacpp 1.5-SNAPSHOT, so merge only after #7296.

farizrahman4u added some commits Mar 25, 2019

@farizrahman4u farizrahman4u requested a review from agibsonccc Mar 27, 2019

size *= d;
}
Pointer ptr = nativeOps.pointerForAddress(address);
ptr = ptr.limit(size);

This comment has been minimized.

Copy link
@agibsonccc

agibsonccc Mar 28, 2019

Member

@farizrahman4u can you resolve this if this concern is fixed?

farizrahman4u added some commits Mar 28, 2019

@farizrahman4u

This comment has been minimized.

Copy link
Member Author

commented Mar 28, 2019

@agibsonccc that buffer creation routine works and we have tests for all precisions. Also we use the same thing in jumpy.

@agibsonccc

This comment has been minimized.

Copy link
Member

commented Mar 29, 2019

@maxpumperla are you satisfied with this?

@maxpumperla
Copy link
Contributor

left a comment

LGTM, my previous concerns have been addressed. 👍

@farizrahman4u farizrahman4u merged commit f0014aa into master Apr 1, 2019

0 of 2 checks passed

continuous-integration/jenkins/pr-head This commit cannot be built
Details
Codacy/PR Quality Review Not up to standards. This pull request quality could be better.
Details

@farizrahman4u farizrahman4u deleted the fr_datavec_python branch Apr 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.