Data key thoughts about icu4x HOT 5 OPEN

Initial thoughts:

1. OK with Proposal 1
2. This is a somewhat radical change to the mental model, but I can buy into it because I've never been a huge fan of the term "key", and you're right that "data marker" and "data key" refer to very similar concepts
3. I agree with the problems you laid out, but I also feel that the directory-like structure has been useful to us. I think if we did Proposal 3 we should do the variant where we use the fully qualified marker name, maybe eliding the "provider" since it will be the same for all markers. icu_list/AndListV1Marker
4. I think I do prefer inlining the fields to the DataMarker trait directly.

from icu4x.

We could have both inlined const fields and a helper DataMarkerInfo

pub trait DataMarker: DynamicDataMarker { 
  const NAME: DataMarkerName;
  const IS_SINGLETON: bool;
  const FALLBACK_CONFIG: LocaleFallbackConfig;
}

#[non_exhaustive]
pub struct DataMarkerInfo {
  pub name: DataMarkerName;
  pub is_singleton: bool;
  pub fallback_config: LocaleFallbackConfig;
}

impl DataMarkerInfo {
  pub fn from_marker<M: DataMarker>() -> Self { /* ... */ }
}

pub trait DynamicDataProvider<M: DynamicDataMarker> { load(&self, DataMarkerInfo, DataRequest) }

But I guess if you do that you might as well just make DataMarkerInfo a const field, as in Proposal 3.

from icu4x.

• @Manishearth - Overall I like merging the names. But the "Marker" name is overloaded.
• @sffc - Marker is a common name that's used for the type-system level objects in Rust. I think there's some risk of overloading "marker" but I'm not too worried. THere are ways to make clear whether a thing is a DataMarker or some other type of marker.
• @sffc - Maybe we can talk about the path structure.
• @robertbastian - https://github.com/unicode-org/icu4x/blob/main/provider/datagen/src/registry.rs
• @sffc - The Rust types already have a hierarchy. It would be nice to reflect that hierarchy in the string representation. But we can probably cut off the icu:: and the ::provider:: and then snake case the rest.
• @robertbastian - I don't like modifying the rust path, because if you then get such a string in an error message, you still have to reverse the mapping, even if it's a more regular mapping than now
• @robertbastian - We can use the Rust stringify! macro if we keep the exact same path. It also makes them more searchable.
• @robertbastian - There's also #4193

Summary of additional discussion later:

• @robertbastian values converging on a single identifier. The dueling identifiers (Rust type name and string path) harm understandability, usability, and searchability.
• @sffc is favorable toward using a single identifier. @Manishearth is neutral or slightly favors having two identifiers since they service different use cases.
• @sffc values hierarchy and brevity. The hierarchy enables use cases such as filtering and diagnostics, and it makes it easier and less error-prone to write custom datagen logic. The brevity directly impacts binary size.
• @sffc puts a higher value on these properties than on the Rust names and would prefer names such as DateTime_Year_Chinese_V1 which are both hierarchical and brief, despite not being conventional Rust names. @sffc argues that these are weird types that only appear in trait bounds so it is fine to establish a new paradigm for how we name them. "V1Marker" is already a paradigm; we're swapping one for another.
• @robertbastian and @Manishearth put a higher value on Rust conventional type names. @robertbastian points out that filtering can still be implemented using custom application logic to parse the camel case. @Manishearth acknowledges that filtering gets more difficult.
• @robertbastian proposed icu::datetime::data::year::ChineseV1 as the identifier. @sffc is potentially okay with this compromise, since it has hierarchy and filterability, but is disappointed that it sacrifices brevity.

from icu4x.

But the "Marker" name is overloaded

to expand on this a little bit (but not to rabbithole too deep because I don't care about "Marker" strongly): I find Marker to be a good default, but it's nice when it can be replaced with a useful concept name instead. I think we may have options here.

one thing I proposed in that discussion that isn't included here was encoding the heirarchy in the trait, so you can have something like:

impl DataMarker for DaySymbolsV1Marker {
   const COMPONENT = "datetime";
   const SUBCOMPONENT = Some("symbols");
}

from icu4x.