Develop issue91: National request for table size change #96
Conversation
…t when it is long.
…nts from biology group.
xiaoliz0
changed the title
marrip
left a comment
marrip
left a comment
There was a problem hiding this comment.
Nice work! I still see some potential for improvement. Please let me know what you think. I can also come up with some suggestions if you want
| if_print_rowNo = True | ||
| table8_column_width = [0.54, 0.96, 0.96, 0.51, 0.73, 1.12, 2.26, 0.79, 0.81, 0.53] | ||
| slide8_table_nrows = insert_table_to_ppt(slide8_table_data_file,slide8_table_ppSlide,slide8_table_name,slide8_header_left,slide8_header_top,slide8_header_width,slide8_table_left,slide8_table_top,slide8_table_width,slide8_table_height,slide8_table_font_size,slide8_table_header,output_ppt_file,if_print_rowNo,table8_column_width) | ||
| table_max_rows_per_slide = int(cfg.get("INPUT", "table_max_rows_per_slide")) - 1 |
There was a problem hiding this comment.
why do we substract 1 here? Is it because of the header?
| slides_list = list(slides) | ||
| slides.remove(slides_list[7]) | ||
| slides.insert(12,slides_list[7]) | ||
| slides.insert(slide_count + 1,slides_list[7]) |
| return data_nrows | ||
|
|
||
|
|
||
| def insert_table_to_ppt_end(table_data_file,slide_n,table_name,left_h,top_h,width_h,left_t,top_t,width_t,height_t,font_size,table_header,output_ppt_file,if_print_rowNo,table_column_width,table_max_rows_per_slide): |
There was a problem hiding this comment.
I see that the function is similar to insert_table_to_ppt - my first question would be, is it possible to combine them and turn on the max rows per slide specifically when needed instead of having similar business logic in two functions - another option would be to refactor the original function and reuse the blocks in both functions that are reusable. Let me know what you think.
I also feel like there are so many input variables, which makes it hard to see all the different values that go in - would it make sense to maybe group some of them into dicts? It might make sense to define a class and add some helper functions to it?
Finally, the function should have unit tests as we have discussed in the last workshop. Would it be possible to add different test cases to make sure it always works as expected?
National request: Adjust tables in report so long tables are split over multiple slides.