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

Rename genrows()? #698

Closed
bluss opened this issue Sep 4, 2019 · 2 comments · Fixed by #872
Closed

Rename genrows()? #698

bluss opened this issue Sep 4, 2019 · 2 comments · Fixed by #872

Comments

@bluss
Copy link
Member

bluss commented Sep 4, 2019

.genrows() has an unfortunate name. It's supposedly the generalized rows of an array. The reason for the tricky name, was that .rows() was taken - by the row count.

We could rename rowsrow_count and genrowsrows to solve this, ideally with deprecations, so in a two version cycle.

rows() is a more natural name for the ndproducer and iterable for rows, for example because the user doesn't have to care that it's “generalized” rows if they only use 2D arrays — they don't need to know and it doesn't matter.

The main problem with the genrows name is that it is non-intuitive to guess to type and guess what it means.

Note that we chose the .rows() name for the row count to match the sprs crate, but I think we can choose to go our own way here, if we have a good reason.

An alternative to the row_count name is nrows() — anyone have a survey of what other crates prefer apart from sprs?

NOTE: The same story applies to gencolumns.

@jturner314
Copy link
Member

I agree that .genrows() has an unfortunate name. It's funny, I've always read it as "generate rows" instead of "generalized rows", which confused me because none of the other iterator/producer methods had the gen prefix. .rows() does seem like a better name.

Other crates:

  • nalgebra: nrows and ncols are the counts. (rows and columns are slicing operations. row_iter and column_iter are the iterators.)

  • rulinalg: rows and cols are the counts. (row_iter and col_iter are the iterators.)

  • sprs: rows and cols are the counts.

My preference for the counts is nrows and ncols/ncolumns because these names are clear while being shorter than *_count.

@LukeMathWalker
Copy link
Member

I agree that .genrows() has an unfortunate name. It's funny, I've always read it as "generate rows" instead of "generalized rows"

Same here 😅

My preference for the counts is nrows and ncols/ncolumns because these names are clear while being shorter than *_count.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants