Nov2020 otn features - [merged] #161

Closed jdpye closed 2 years ago

In GitLab by @ryangosse on Dec 2, 2020, 13:08

Merges nov2020-otn-features -> dev

These are all the features I want to be added to the new version of glatos

In GitLab by @ryangosse on Dec 2, 2020, 13:33

unmarked as a Work In Progress

In GitLab by @ryangosse on Dec 2, 2020, 13:33

assigned to @chrisholbrook and unassigned @ryangosse

In GitLab by @ryangosse on Dec 7, 2020, 20:06

added 1 commit

921a3414 - Fixed my last name

Compare with previous version

In GitLab by @chrisholbrook on Dec 8, 2020, 18:05

@ryangosse this errors because only one sheet in hfx_deploy_simplified.xlsx If that's the correct file, then should be this?

#' deploy <- prepare_deploy_sheet(deploy_path, 5, 1)

In GitLab by @chrisholbrook on Dec 8, 2020, 18:06

added 2 commits

4502ea1b - hotfix doc issue; replace `in_file` with 'in_file' (backticks) to single quote
067e6ada - update docs

Compare with previous version

In GitLab by @chrisholbrook on Dec 8, 2020, 18:33

@ryangosse Two issues here.

in read_excel, skip is applied before anything is read, so it will omit the header. In the example above (when start = 5), the data on the 4th row will become header. So if you really want start row flexibility, then maybe read in the column names first, then specify those, as suggested below.
Since skip includes the header, if we want the first record to be the 5th data record, then we need to set skip = start instead of skip = 1, as suggested below.

    #get row names to skipping when start > 1
    col_names <- names(readxl::read_excel(path, n_max = 1))
    sheet <- readxl::read_excel(path, sheet = sheet, skip = start,
                                col_names = col_names)

In GitLab by @ryangosse on Dec 16, 2020, 15:52

changed this line in version 4 of the diff

In GitLab by @ryangosse on Dec 16, 2020, 15:52

added 1 commit

89282ffd - Apply suggestion to R/load-prepare_deploy_sheet.R

Compare with previous version

In GitLab by @ryangosse on Dec 16, 2020, 15:55

resolved all threads

In GitLab by @ryangosse on Dec 16, 2020, 15:55

changed this line in version 5 of the diff

In GitLab by @ryangosse on Dec 16, 2020, 15:55

added 1 commit

2d7411a6 - Apply suggestion to R/load-prepare_deploy_sheet.R

Compare with previous version

In GitLab by @ryangosse on Dec 16, 2020, 16:12

added 1 commit

bd5990b5 - Fixed some minor bugs for the ATT crosswalk from OTN functions

Compare with previous version

In GitLab by @ryangosse on Dec 16, 2020, 16:22

Yes, that's the correct file, I just messed up when copying and pasting.

In GitLab by @chrisholbrook on Apr 23, 2021, 21:14

Default value start = 1 does not work with the example file otn_nsbs_tag_metadata.xls. If the example file is a standard format, then the default value should be start = 5.

In GitLab by @chrisholbrook on Apr 23, 2021, 21:15

Default value sheet = 1 does not work with example file otn_nsbs_tag_metadata.xls. If the example file is a standard format, then the default value should be sheet = 2.

In GitLab by @chrisholbrook on Apr 23, 2021, 21:49

added 3 commits

5a0be7f8 - update docs
3cc2b54d - fix typo in help example
0299fba8 - fix typos in help; several detection replaced with deployment

Compare with previous version

In GitLab by @chrisholbrook on Apr 23, 2021, 22:31

I would prefer if object sheet here were not named the same as argument sheet.

In GitLab by @chrisholbrook on Apr 23, 2021, 22:31

This breaks if start is not equal to the line number with headers. Is that intended? If so, describe that here. If this is intended to provide some other flexibility (skipping first n rows of data), then it breaks; e.g., try tags <- prepare_tag_sheet(tag_path, start = 6, 2)

In GitLab by @chrisholbrook on Apr 23, 2021, 22:31

Note that this returns a list, not a vector, result is complex structure of return object... run the example and then str(tags)... est_tag_life is a list of 20, should be integer vector.

In GitLab by @chrisholbrook on Apr 23, 2021, 22:31

Bunch of issues in the doc here where detection was used in place of deployment; obviously carryover from earlier copy-paste. Fixed in commit 0299fba8d7baf371c84ad4c886982f4482d22eb9.

In GitLab by @chrisholbrook on Apr 23, 2021, 22:33

added 2 commits