Sarah comments

[wright13]

Note: some of these comments may be redundant with existing issues

• Functions need to be exported
• If secure connection to IRMA fails, remind user to get on NPS network
• What's the scope of this package? Right now it's essentially data pkg utils but I can think of LOTS of functions that could fall under the "NPS utils" umbrella.
• get.DataPackage
  • Should accept destination dir as an argument (default to here::here("data", ReferenceID))
  • How do we expect data package holdings to be structured? Consider searching holding info for zip file with "datapackage" in the name so it doesn't break if other files are present
  • Need to troubleshoot secure option
• getUnitCode
  • What does "xmlns: URI NRPC.IrmaServices.Rest.Unit is not absolute" mean? If this is useful info, consider providing some context, otherwise, omit it?
• get.parkCode
  • Is this useful? Only returns National Parks (not nat'l monuments, etc).
• The functions in getParkUnitInfo seem redundant - can we just use get.unitInfo?
  • If we want to keep getUnitCode, maybe just have it return a named vector where names are unit names and values are unit codes?
    • If we do this, consider adding getUnitName() function to go the other direction (code -> unit name)
• get.unitInfo
  • Default LifeCycle to "Active"
  • Consider using match.arg to validate some args
  • Consider modifying to accept vectors of length > 1 as filters (e.g. Code = c("JOTR", "DEVA"))
• get.RefInfo
  • Is there a reason this subset of reference info was chosen to be made available (as opposed to things like copyright, files/links, bibliography, etc)?
  • Would it be better to just return all ref info as a (tidied) list instead of specifying the field?
• load.dataPackage
  • What if we load metadata inside this function and use that to set dataframe column types? I have some code that does this. It's probably worth returning the metadata as well.
  • I recommend accepting the data package dir as an argument instead of holding ID. It would be useful to have get.DataPackage return the dir it wrote the data package to so that users can call get.DataPackage %>% load.dataPackage
• load.dataPackageList
  • What's the intended use of this fxn?
• validate.datapackage
  • Should this live here or in DPchecker?