I agree that consistency is good. I prefer underscore naming, I find it more 
readable and it's what PEP 8 recommends. I think number_of_facets is a bit too 
long for my tastes (mostly because we try to adhere to line length limits and 
long names make that annoying). If I were writing that code I'd probably call 
it num_facets.

Of course how much of a problem a long name is depends on how often it's used. 
If we renamed the matrix nrows and ncols to number_of_rows and 
number_of_columns then it would be very annoying to write an if statement to 
check the matrix size against some other values and have that line fit in 80 
characters (sure you can wrap it, but I think that hurts readability). Perhaps 
number_of_facets is not used so much that this is an actual problem. But I 
think num_facets is a happy medium that is both clear and short enough to not 
be annoying, which means that naming scheme can be used consistently.

For methods that are used very frequently (ncols, nrows, ngens, etc.) I think 
it's probably fine to leave them as-is. We could add aliases for num_cols, 
num_rows, etc. if we want.

I recently added functionality to our documentation builder 
(https://github.com/sagemath/sage/pull/40753) to detect aliased functions and 
methods and not duplicate them in the HTML docs (it now says "alias of [link to 
original]", before it just duplicated the docstring including all the examples) 
so bloating the documentation with aliases isn't really a problem anymore (in 
case someone was worried about that).

One thing I would note is that changing the name of nfacets is technically a 
breaking change and there should be an alias for the old name with a 
deprecation warning in place for however long our depreciation period is.

Also, thank you Frédéric for all the refactoring and code cleanup work you've 
been doing!

Vincent Macri (he/him)

-- 
You received this message because you are subscribed to the Google Groups 
"sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion visit 
https://groups.google.com/d/msgid/sage-devel/TO1PPFB4708EDB7FDDDE96CEEDB03DEF318F71CA%40TO1PPFB4708EDB7.CANPRD01.PROD.OUTLOOK.COM.

Reply via email to