ToucanToco / fastexcel

A Python wrapper around calamine
http://fastexcel.toucantoco.dev/
MIT License
116 stars 6 forks source link

Should `total_height` ignore offset due to header rows? #312

Open bzm3r opened 2 weeks ago

bzm3r commented 2 weeks ago

Suppose I want to get the total height of a sheet, including header rows. Currently, the only way to do this is to load_sheet with header_row=None and then to get the total_height, because total_height currently is sensitive to the number of header rows: https://github.com/ToucanToco/fastexcel/blob/aa0ab7723ed27247aaa35a9d36969668e715050d/src/types/python/table.rs#L234

Should total_height also include the number of header rows (i.e. be insensitive to the number of header rows)?

lukapeschke commented 1 week ago

I agreee that the name is confusing, but we cannot break existing behaviour. What we could do is deprecating the current height, width, total_width and offest properties on the ExcelSheet and ExcelTable types, and introduce a new property which would contain all the shape-related information, including:

Naming is not one of my strengths, so I'm open for suggestions on this :)

@PrettyWood what are your thoughts on this ?