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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Static type checking with PhantomData #41

Merged
merged 6 commits into from Aug 3, 2018

Conversation

kngwyu
Copy link
Member

@kngwyu kngwyu commented Jul 28, 2018

Currently, this code

let v: Vec<i32> = vec![1, 2, 3];
let pyarray = PyArray::from_vec::<i64>(py, &np, v);

causes panic.
But, isn't it annoying to give a type annotation like from_vec::<i64>, and isn't it really user-friendly to cause panic?
So, this PR introduces PyArray<T> instead of PyArray.
By this change, when we write

let v: Vec<i32> = vec![1, 2, 3];
let pyarray = PyArray::<i64>::from_vec(py, &np, v);

, we can get compile error.

@termoshtt
This is a breaking change so I want to hear your opnion 馃檪

@kngwyu kngwyu requested a review from termoshtt July 28, 2018 06:22
@termoshtt
Copy link
Collaborator

This is a breaking change so I want to hear your opnion 馃檪

I totally agree with this. It is great feature :)

I think one may want to check the dimension of the array statistically like PyArray<i32, Ix2> (Ix2 from ndarray crate). What do you think about it? It must be also breaking change.

@kngwyu
Copy link
Member Author

kngwyu commented Jul 29, 2018

one may want to check the dimension of the array statistically

It's a difficult decision, but I feel it's too restrictive to give dimension parameter to PyArray.
It'll work well in Rust -> Python scenario, but how about vice versa?
By this PR, we can omit Type Parameter of data type in Rust->Python conversion, like

let vec2 = vec![vec![1, 2, 3]; 2];
let pyarray = PyArray::from_vec2(gil.python(), &np, &vec2).unwrap();

But, in Python -> Rust scenario, we still have to give type parameter and do dynamic type checking, like

 let pyarray: &PyArray<i32> = gil
        .python()
        .eval("np.array([1, 2, 3], dtype='int32')", Some(&dict), None)
        .unwrap()
        .extract()
        .unwrap();

(full example and where we do dynamic type cheking)
So in this case, if we add dimension parameter, users have to specify it when they get PyArray from python. I think it'll be somewhat confusing for users who are not familiar to ndarray crate.

@kngwyu kngwyu mentioned this pull request Aug 2, 2018
@kngwyu kngwyu merged commit 5c853af into PyO3:master Aug 3, 2018
@kngwyu kngwyu deleted the static-type-check branch August 3, 2018 07:38
@kngwyu kngwyu mentioned this pull request Sep 6, 2018
@kngwyu kngwyu mentioned this pull request Oct 9, 2018
1 task
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.

None yet

2 participants