Closed chainsawriot closed 1 year ago
The variable sheet has different meanings in the codebase.
sheet
Other than as the argument of read_ods and write_ods
read_ods
write_ods
it is an integer after standisation aka sheet_index in some places
sheet_index
https://github.com/ropensci/readODS/blob/f8fc6570957c3587254dff340e760d4b939d9c79/R/read_ods.R#L195-L196
here, the sheet in the name convert_df_to_sheet means a "sheet file"
convert_df_to_sheet
https://github.com/ropensci/readODS/blob/f8fc6570957c3587254dff340e760d4b939d9c79/R/write_ods.R#L42-L47
In the C++ code, there is type but can still be confusing:
https://github.com/ropensci/readODS/blob/f8fc6570957c3587254dff340e760d4b939d9c79/src/write_sheet_.cpp#L128
https://github.com/ropensci/readODS/blob/f8fc6570957c3587254dff340e760d4b939d9c79/src/read_ods_.cpp#L10
May I propose this:
sheet_name
sheet_file
Please keep using whatever works. I will clean this up after feature freeze for the next CRAN release.
The variable
sheet
has different meanings in the codebase.Other than as the argument of
read_ods
andwrite_ods
it is an integer after standisation aka
sheet_index
in some placeshttps://github.com/ropensci/readODS/blob/f8fc6570957c3587254dff340e760d4b939d9c79/R/read_ods.R#L195-L196
here, the sheet in the name
convert_df_to_sheet
means a "sheet file"https://github.com/ropensci/readODS/blob/f8fc6570957c3587254dff340e760d4b939d9c79/R/write_ods.R#L42-L47
In the C++ code, there is type but can still be confusing:
https://github.com/ropensci/readODS/blob/f8fc6570957c3587254dff340e760d4b939d9c79/src/write_sheet_.cpp#L128
https://github.com/ropensci/readODS/blob/f8fc6570957c3587254dff340e760d4b939d9c79/src/read_ods_.cpp#L10
May I propose this:
sheet
- use only in R to denote an argument that can either be a name or indexsheet_index
- use in R and C++ to denote an integer representing the sheet index, e.g. 2sheet_name
- use in R and C++ to denote a string representing the sheet name, e.g. "Sheet1"sheet_file
- use in R and C++ to denote the often throwaway XML filePlease keep using whatever works. I will clean this up after feature freeze for the next CRAN release.