`val.meter` documentation feedback pass

[dgkf]

Opening an issue to collect general feedback on val.meter vignettes and function documentation.

Please feel free to lend any comments - large or small; comments appreciated for clarity, spelling, flow; anything!

The easiest place to read through is probably the github pages website. If you give a suggestion, please copy enough context so that I can find the text you're referring to, or if you feel comfortable, finding the snippet in the code to link directly to the source.

[ramiromagno]

@dgkf : First of all, amazing internal package design. Still learning from it...

Frivolous remarks

Blurry text with print method of S7 objects

There's no background for code blocks in light mode, is this intended?

Typos

Each resourceS should be able (...)

val.meter/R/class_resource.R

Line 5 in 9454a94

#' repository listings. Each resources should be able to produce a downloadable

On impl_data()

• When impl_data() is introduced in articles/adding-metric it would be nice to have a cross-reference, perhaps to the internals vignette, for more details.
• I think that for these functions with these side effects (e.g. impl_data()) they are perhaps a bit too silent, requiring you to call metrics(all = TRUE) to see their effect. My suggestion: in addition, and if in interactive mode, then print details about the successful registration.

Package data

The expression package data can be misunderstood for data bundled in a package instead of the package metadata. I also prefer a short name but it might be worthwhile thinking of an alternative expression. And when reading about Implementing data that same vagueness feeling surfaces... :)

Metric definition

In articles/adding-metric, when introducing metric data constraints, three questions: (i) is the atomic constraint following is.atomic() strictly? (ii) does it have to be a scalar? and (iii) special values like NA, Inf, NaN, etc.. are allowed?

Misplaced code chunk?

val.meter/vignettes/repository-integrations.Rmd

Lines 17 to 21 in 9454a94

	```{css}
	[data-bs-theme="dark"] main img {
	filter: invert(1);
	}
	```

[Zhenglei-BCS]

In my settings:

[ramiromagno]

@Zhenglei-BCS: thanks for checking! As it seems mostly on my environment only I will investigate a bit more on my end.

[dgkf]

There's no background for code blocks in light mode, is this intended?

Yes... sorta. I removed the border lines and in light mode there's no difference in code block and body backgrounds.

Blurry text with print method of S7 objects

@Zhenglei-BCS @ramiromagno - This uses cli::style_dim, but I think some terminals implement this equivalent to "blur". So even if it's not an issue for everyone, I think I was too ambitious to use exotic formatting and we should use something more conventional to avoid this.

My suggestion: in addition, and if in interactive mode, then print details about the successful registration

That's a great suggestion and an easy addition. 👍

The expression package data can be misunderstood for data bundled in a package instead of the package metadata

Oh my god yes. Naming things is hard 😞 - happy to hear alternatives. I floated between "info" (used internally) "data" (very, very general and maybe misleading as you point out) and "metadata". I haven't landed on one that I'm completely happy with.

In articles/adding-metric, when introducing metric data constraints, three questions: (i) is the atomic constraint following is.atomic() strictly.

Yes, specifically, it uses S7 to check that it inherits from the class_atomic class union. There are no further constraints, so this includes all the special numeric values and permits vector values. Vector values and NA have been useful so far... I'm not sure if Inf and NaN would be problematic.

Misplaced code chunk?

Yeah, forgot to hide the code. It's just used for improving plot readability in dark mode by inverting the graphic. It works alright for existing plots, but it's definitely not a universal solution.