Skip to content

[ENHANCEMENT] Consider renaming eachblockstoredindex to blockeachstoredindex #72

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

Open
mtfishman opened this issue Mar 4, 2025 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@mtfishman
Copy link
Member

mtfishman commented Mar 4, 2025

I think we should consider renaming eachblockstoredindex to blockeachstoredindex. I think that would be closer to the naming convention used in BlockArrays.jl, though I've been going back and forth about it.

@mtfishman mtfishman added the enhancement New feature or request label Mar 4, 2025
@mtfishman mtfishman self-assigned this Mar 4, 2025
@mtfishman
Copy link
Member Author

@lkdvos @ogauthe curious to hear your thoughts on this, I've been going back and forth about this for a while.

@mtfishman mtfishman changed the title [ENHANCEMENT] Rename eachblockstoredindex to blockeachstoredindex [ENHANCEMENT] Consider renaming eachblockstoredindex to blockeachstoredindex Mar 4, 2025
@lkdvos
Copy link
Contributor

lkdvos commented Mar 4, 2025

more options is probably not what you want, but also possible: eachstoredblockindex

I see what you mean about block_ being the convention in BlockArrays.jl, but blockeachstoredindex does read a bit funny, while the other feel a bit more language-friendly

@mtfishman
Copy link
Member Author

mtfishman commented Mar 4, 2025

more options is probably not what you want, but also possible: eachstoredblockindex

I also considered that, but I didn't want the term blockindex to be conflated with the BlockArrays.BlockIndex type, which has a different meaning (i.e. it refers to an index within a block, not a block location itself).

I see what you mean about block_ being the convention in BlockArrays.jl, but blockeachstoredindex does read a bit funny, while the other feel a bit more language-friendly

Indeed, those are basically the main considerations. Anyway, I'll leave this open for now and we can think about it, I was hoping to complete low hanging breaking changes soon so that some of these choices don't get too ingrained but this one would be easy to change over if we wanted to.

@ogauthe
Copy link
Collaborator

ogauthe commented Mar 5, 2025

I agree eachstoredblockindex recalls too much BlockIndex. I have a weak preference for the current eachblockstoredindex, I like that it starts with each such that I easily associated it with eachindex that plays a similar role. I find it harder to parse blockeachstoredindex. Overall I value convenience over strict naming convention when we have the freedom to name a new function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants