Documentation

[kinto-b]

Looks good to me. Only thing that should be fixed before this is merged are the links in the vignette.

Actually the whole "Categories" section probably needs a revamp given that it doesn't really align with the "Expectations" section of the "Reference" page.

But looking at the "Reference" page now, it's still not organized in the best possible way.

For instance, do the "Expectations - auto-generated" really constitute a reasonable category? I now realise it made more sense before as "value expectations", but that's still not perfect. Perhaps "Expectations - format" for pattern, length, date, and so on, "Expectations - values" for value and range, and something else for text missingness (which are potentially questionable given the existence of expect_base).

Additionally, the descriptions are inconsistent. Some have "checks" in the description, others don't, etc.

Originally posted by @kinto-b in #45 (review)

[gorcha]

Ta! Also see #34

[kinto-b]

Oh missed that, probably could've just piggy backed. I was thinking about looking at this today and doing a PR. I reckon it's a good idea to split up the functions across more man pages. More user friendly that way, and be able to write up more pointed documentation.

I'm thinking relabeling for consistency:

• Expectations: consistency
• Expectations: comparisons
• Expectations: generic helpers
• Expectations: exclusivity
• Expectations: labels
• Expectations: proportions
• Expectations: uniqueness

And breaking up the auto-generated ones

• Expectations: values -- expect_values(), expect_range()
• Expectations: patterns -- expect_regex(), expect_max_length()
• Expectations: dates -- expect_date_yyyy(), expect_date_yyyymm(), expect_date_yyyymmdd()
• Expectations: text -- expect_text_miss(), expect_text_nmiss()

The chk_*() family will be broken up to match:

• Checks: values
• Checks: patterns
• etc.

And I'll make sure there's at least one example for each exported function.

Thoughts?

[kinto-b]

Also, perhaps it's best to use @describeIn for man pages that document functions which require separate description, like the conditional expectations

We end up with a cordoned off Functions section:

[gorcha]

Awesome, that documentation structure looks great!

The only other thought I had is combining the corresponding expect and chk functions into the one man page, but that might be a bit weird because the function signature and use case are quite different, what do you think?

Personally I prefer @rdname to @describeIn but I can't actually remember why so not super fussed. Does @describeIn still include separate usage entries?

[kinto-b]

@gorcha Sweet :)

Yeah, it seems to do exactly rdname does only with the addition of that Functions section:

I think it's better to keep them separate and link them using @seealso and/or by mentioning the chk_*() function in the corresponding expect_*() man page.

[kinto-b]

@gorcha slightly off topic, but I'm just documenting the chk_text_miss functions --- what exactly is the envisioned use-case for these?

[kinto-b]

@gorcha Do you think it's worth addressing the inconsistency between expect_regex and chk_pattern ?

[gorcha]

Good question! They're related to some of the text cleaning in the sample cleaning functions, essentially for making sure that any missing placeholders that might have crept in from other systems are removed.

Not 100% sure of their usefulness, it's almost a special case of chk_values() or !chk_values().

[gorcha]

Also yes definitely re: chk_pattern() - I think it should be renamed to chk_regex() since that seems clearer and more direct to me.

[kinto-b]

Cool, have opened a new issue for chk_pattern().

Hm, I can't really come up with a good example to demonstrate their usefulness. The only thing I can think of is:

clean <- ifelse(chk_text_miss(dirty), "", dirty)

But that doesn't really seem like a job for testdat. And it's debatable whether that's better than the user defining their own missing text based on the systems their interfacing with (e.g. it's risky to remove "0" in a less than explicit way).

I can't really think of anything for chk_text_nmiss().

[gorcha]

Yeah agreed. I think chk_text_nmiss() (or expect_text_nmiss()) is potentially useful for expecting that these kinds of values don't exist, but a complement for (chk|expect)_values() is probably a better way to do this.

[gorcha]

It might be good to have a more direct expect_not_missing() function as well (basically expect_base(var, TRUE))

[kinto-b]

I'm gonna open up an issue for it specifically