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

findIndex should not be in Data.Massiv.Vector #123

Open
dschrempf opened this issue Dec 12, 2022 · 2 comments
Open

findIndex should not be in Data.Massiv.Vector #123

dschrempf opened this issue Dec 12, 2022 · 2 comments

Comments

@dschrempf
Copy link

dschrempf commented Dec 12, 2022

Hi! Thank you for this great package!

I just wrote my own findIndex function for general arrays before finding a similar function in Data.Massiv.Vector. I think the function findIndex should be moved to a more general module!

Thank you!

EDIT: Or, maybe. move the function and add a specialized one to Data.Massiv.Vector?

@lehins
Copy link
Owner

lehins commented Dec 12, 2022

The function itself is defined and exported from: Data.Massiv.Array.Manifest.findIndex

-- | /O(n)/ - Perform a row-major search starting at @0@ for an element. Returns the index
-- of the first occurance of an element or `Nothing` if a predicate could not be satisifed
-- after it was applyied to all elements of the array.
--
-- @since 0.5.5
findIndex :: (Index ix, Manifest r e) => (e -> Bool) -> Array r ix e -> Maybe ix

It is merely re-exported from Data.Massiv.Vector and Data.Massiv.Array modules. I am not sure there is benefit of removing re-export and redefining a specialized version in Data.Massiv.Vector

I think a better solution would be instead of re-exporting the full Data.Massiv.Array.Manifest re-export functions from the module in Data.Massiv.Array in organized manner.

There is quite a bit documentation improvement massiv could use. PRs are always welcome.

@dschrempf
Copy link
Author

I see, thank you for the explanation.

I just didn't find this or any similar function in the documentation of Data.Massiv.Array. Yes, an organized re-export would be the preferred solution...

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