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

update document to reflect the array ordering change and make it consistent in the pkg #54

Open
hubutui opened this issue Jun 11, 2020 · 2 comments

Comments

@hubutui
Copy link
Contributor

hubutui commented Jun 11, 2020

Correct me if I get it wrong. Accoring to c8e496f, now the array is in shape of [n_samples, n_features], just like scikit-learn or other python packages. So,

x : [n_samples, n_features] `float32` `ndarray`
One column per data vector (e.g. a SIFT descriptor)

I think you mean

One row per data vector (e.g. a SIFT descriptor)

And there are some inconsistent usage, say vlad, when I update the commits from it use different array ordering. Now we have two different array ordering, which will make users confused. We need to make it consistent.

@patricksnape
Copy link
Contributor

As you can see from #20 there was a lot of discussion about which ordering was the "right" ordering. And for the fisher module it was agreed in that PR that the [n_samples, n_features] was appropriate for the underlying call to vlfeat.

Obviously it would be best to standardize this and test that the underlying modules actually act correctly - but this is beyond my personal scope as I don't use any of the modules recently contributed. Again - happy to try and generally review and cut releases but don't have the scope to actually make this wrapper feature complete.

@patricksnape
Copy link
Contributor

First part addressed in #59

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

No branches or pull requests

2 participants