Improve metadata docs

[benjeffery]

I've started an issue on this as there has been some discussion and wanted a central place for people to make suggestions, rather than on other PRs.

e.g.
#595 (comment)

[benjeffery]

struct codec parameters need more explaining for sure.

[benjeffery]

struct default array length is L not Q.

[bhaller]

Hi folks. Here's a read-through from a naive user who only just barely grasps what any of this means, and is pretty much Python-illiterate, so you can just ignore my comments if they are too clueless. :->

Typo: "A good to guide to"

Regarding: "The metadata for each entity is described by a schema for each entity type." I feel like at this point it needs to be clarified who defines the schema, and whether it is fixed for each entity type or not. A sentence like "The metadata schema for each entity type is typically defined by the software that is generating the tree sequence; it will vary among software packages depending upon the information they have and wish to preserve. A user of tskit could also, in principle, define their own metadata schema in order to associate different or additional information with entities, but more typically they will work with the metadata schema they receive from the tree-sequence-generating software they use." Something like that, addressing where the schema comes from, who defines it, whether it is always the same for a given entity, whether it can be changed, etc. What the usage pattern is that surrounds all this.

Typo: "by any third-party" (no hyphen since "third party" is a noun, not an adjective; as an adjective, "third-party software", it gets a hyphen :->)

Re: "It is possible to define a custom codec using tskit.register_metadata_codec(), however this should only be used when necessary as downstream users of the metadata will not be able to decode it without the custom codec." I'm not sure I understand this point. If you use a custom codec, the underlying data will still be binary, and could still be decoded with a JSON or struct codec that matches the format of the data, no? It just can't be decoded by whatever the custom codec is unless the downstream user has that custom codec? Or maybe there can only be one codec declared for a given metadata schema? Might need clarification. What I'm driving at is: one might want to define a custom codec that decodes metadata into some kind of spiffy custom Python objects that do fancy things, but the underlying binary data might also be representable with a struct codec that matches the format, allowing access to the metadata without all the spiffy extra features provided by the custom codec. Is this sort of dual nature allowed? Or am I completely misunderstanding what is meant by a "custom codec"?

Re: "Endian-ness is fixed at little endian so cannot be specified". Apropos of this: honestly I have no idea what SLiM does with endianness. I think all the platforms I've ever tested it on have the same endianness so there are probably lots of bugs here. Nobody has ever complained, however. Perhaps it would only bite people if they moved .trees files from one platform to another, I'm not sure – I have never really understood endianness very well, since SLiM is the first cross-platform software I've ever worked on. Anyway, this seems like a reasonable stake, and if SLiM violates it bugs can be filed. :->

"binaryFormat must match ^[cbB?hHiIlLqQfd]|\d*[spx]$": Is that a regex? lol, OK, I guess giving a regex is one way to specify this. :-> But it might be helpful to also explain in words what the restrictions are, and the reasons for them, if that is not too complicated. The regex is, ahem, not the most obvious thing for the average reader.

"For strings, the encoding can be set": I would move this upward, to just below "Strings that are longer than", so that all discussion of string details are together. Also: ok, "utf-8" is the default; should there be a mention of what other values are supported? Is encoding used/enforced by tskit, or is this up to the user? I guess what I'm asking is: when the metadata comes out through the codec and arrives to the user in Python, is string data turned into Python string objects using the specified encoding, or does the user just get a pointer to bytes? If the former, then a link to what encodings are supported would be useful.

"no additional properties not listed in the schema" : What happens if there are extra bytes left over in the binary metadata after the struct schema has interpreted everything? Does that cause an error, or do the extra bytes just get ignored?

"The order that properties are encoded is by default alphabetically by name." This seems a bit surprising. Is there a reason that they are not encoded in the order they are specified in the schema? That seems like it would be simpler, and would remove the need for the index property. But I imagine you have your reasons. :->

"As an example here is a schema using the struct codec...": OK, I guess I'm being clueless here, but the schema that follows says "codec": "json". So doesn't that use the JSON codec, not the struct codec, then?

Typo: "for the each row of the table"

Regarding this schema, I'm surprised to see that accession_number is required but collection_date is not. I thought that the rule "All metadata objects must have fixed properties" for struct codec schemas meant that every property must be present. If I have misunderstood, then that "must have fixed properties" statement needs to be clarified.

Typo: "and raw bytes stored/retrived" (should be "retrieved")

OK, there's what I've got. I didn't follow links to other parts of the doc since I'm not sure what is new and needs review, versus what is pre-existing and doesn't. If this sort of totally-clueless-user feedback is useful to you, feel free to feed me more links and I'll review them, I'm happy to help. Cheers.

[benjeffery]

This is great feedback, thanks @bhaller! Will work this into the next pass.

[petrelharp]

Expanding a bit on my comment linked to above: I'd assume that you have a tree sequence with metadata scheme already in it, and walk through looking at the schemas to see what information is there, and extracting information from the metadata. I think specifying and setting a scheme can be in the more technical documentation at the metadata docs?

[petrelharp]

Note: the documentation for these methods could be more precise:

 |  decode_row(self, row: bytes) -> Any
 |      Decode an encoded row of metadata, note that no validation is performed.
 |  
 |  encode_row(self, row: bytes) -> Any
 |      Encode an encoded row of metadata, note that no validation is performed.
 |  
 |  validate_and_encode_row(self, row: Any) -> bytes
 |      Validate a row of metadata against this schema and return the encoded
 |      representation.

[molpopgen]

• It would be nice to have a mapping from numpy dtypes to their struct encodings in a table.
  The docs at the struct page talk about size in bytes, so people less familiar with
  this stuff may have trouble with 4 = 32. These sizes are very important,
  and it may also help to point out that the Python int behaves very differently
  from C/numpy integer types.
• The Examples section talks about struct codec but then the codec in the code
  block is marked "json"? It is also a bit confusing that the stuff in curly
  braces isn't explicitly described as a dict. I think it would help to
  show the codec being assigned to a variable to make that point very clear.
• The docs are extremely technical, and I find the example of a custom
  codec really hard to parse. The term MsgPack isn't defined, and it is not clear
  what the relationship is between the class MsgPackCodec and the dict in the next
  block. It seems that the 'msgpack' string is crucial here, but one has
  to read quite closely to see that as the common thread.
  There also seems to be an error in the second block, as raw_schema is defined but
  never used?
• The docs aren't following the black style? Single quotes have sneaked in.
• The description of array encoding for the struct codec only contains invalid
  examples. I got confused by this and ended up trying to put my metadata into []
  when they should not have been. This section could use a fully working/correct
  examples.
• false instead of False in the Examples section.
• It took me a while to find the section on decoding the metadata. It'd be nice
  to make that more front and center.
• If you accidentally put index in the items field of an array,
  the schema is considered valid? Don't ask me how I discovered that.

[benjeffery]

This is great stuff, really appreciate the detailed feedback from you all. Sorry you're having to trudge through my mistakes!

[petrelharp]

+1 to more explicitly saying things are dicts.

Also, the index parsing isn't very strict - looks like it doesn't matter if you start from 0 or 1, for instance? It might help to catch bugs if it was strict.

[petrelharp]

The docs don't say how to specify the length of an array, if not by noLengthEncodingExhaustBuffer, as far as I can figure out?

[petrelharp]

Generally, it's great, btw - just minor ways to clarify stuff.