Add extra fields by danpf · Pull Request #36 · rcsb/mmtf

danpf · 2018-08-31T17:34:29Z

This PR gives applications/users places to store their own data that's associate with their mmtf files.

Up for discussion:
Any Convention key:value pairs that you would like to see added.
Any Convention encode types you'd like to add
Do you like the format?

Other conventions to think about adding:

specific colors for different representations

If you are interested there is already a working implementation of this in:
https://github.com/danpf/mmtf-cpp/tree/rcsb_extraData

This is meant to clear up issue:
#32

arose · 2018-08-31T23:55:09Z

spec.md

+
+*Type*: [Map](#types)
+
+*Description*: A field meant to store information on specific bonds.  Must have the same length as [numBonds](#numbonds).


If it must be of same length as numBonds then it can be an array instead of a map, right?

Sorry I will think of a way to rephrase this, the idea is that you have keys with the values being arrays with length numBonds if that makes sense!

So, a map where the keys are integers in [0, numBonds - 1].

One note, using maps is more flexible but does not easily allow for the bespoke array compression techniques.

it would be like:

for something with 5 atoms atomProperties: { "colorList": ["#ff0000", "#008000", "#ff0000", "#008000", "#ff0000"], "representationList": [0, 0, 0, 1, 1] } extraProperties: { "availableRepresentations": {0: "lines", 1: "spheres", 2: "surface", 3: "ball and stick", 4: "cartoon"} }

availableRepresentations in this case doesn't have to be 5, while colorList and representationList do have to have len(5)

@arose You can also find a bigger example in the original discussion thread

From the examples it looks to me the type should be Array (or List) and not Map.

Does it make more sense now @arose ?

arose · 2018-08-31T23:56:31Z

spec.md

+
+*Description*: A field meant to store information on specific bonds.  Must have the same length as [numBonds](#numbonds).
+
+*Convention key:value pairs*


A description of what is meant exactly by 'convention' would be good.

I will also rephrase this, I think I will change this to reserved key:value pairs and add a little explanation at the of the extra data header.

The idea was to have some key: value pairs locked so that you could save things like camera positions/color schemes between applications. (ie everyone use cameraOrientation not app1_camera_orientation and app2_camera_orientation).

gtauriello · 2018-09-03T14:18:06Z

spec.md

+
+*Type*: [Map](#types)
+
+*Description*: A field meant to store information on specific bonds.  Must have the same length as [numBonds](#numbonds).


@arose You can also find a bigger example in the original discussion thread

gtauriello · 2018-09-03T14:21:51Z

spec.md

+*Type*: [Map](#types)
+
+*Description*: A field meant to store any information at all. There are no length restrictions associated with this field.  We
+encourage you to apply our encoding techniques to your application data to reduce file sizes!


I would propose to move this encouragement to the parent section, since it makes a lot of sense to use encoding techniques for any of the other extra data fields too (especially large ones like bondProperties, atomProperties and groupProperties).

spec.md

arose · 2018-10-13T21:16:06Z

I like it, I have one more request for the spec. I think the type of the 'second level' of the maps should be defined more clearly, i.e. Map<String, SECOND_LEVEL_TYPE>.

So atomProperties, bondProperties, groupProperties, chainProperties and modelProperties are maps of type Map<String, Array>. And the array must be of the same length as the corresponding num* filed.

How about adding a note about more specific Map types to the types section and then define the map type exactly for all the *Properties fields?

gtauriello · 2018-10-15T10:03:01Z

I like the idea of specifying the second level type to be some type of list but I would have it as Array|Binary. Meaning that I wouldn't want to enforce in the specs whether extra fields should be encoded as binary objects or not.

danpf · 2018-10-15T15:54:57Z

@arose @gtauriello I think I addressed both of your concerns. Let me know if that is what you had in mind!

gtauriello · 2018-10-15T18:40:24Z

Looks verbose but very clear. I did observe some issues when looking at the full use case example though (see comments below).

gtauriello · 2018-10-15T18:46:58Z

spec.md

+  ...
+  "structureProperties": {
+    "foo_id": "ABC",
+  },


There is an inconsistency with the specs here. We don't have a structureProperties field. I guess that properties at the structure level should go into extraProperties?

At least, I don't see the use case for having a structureProperties field vs adding structure-level properties in extraProperties.

oops good point. Yes I'm ok with just wrapping it all into extraProperties. it would be sort of a useless field.

gtauriello · 2018-10-15T18:55:40Z

spec.md

+
+Example
+
+```


Visual detail: I think you can use ```python for a nice syntax highlighting here. I assume you removed the json highlighting since comments and ... would have looked bad there...

ok I will try that!

gtauriello · 2018-10-15T18:57:26Z

spec.md

+  },
+  "extraProperties": {
+    "pymol_bondTypes": {0: "metal", 1: "single", 2: "double", 3: "triple", 4: "aromatic"}
+  }


One note on type restrictions for extraProperties: the example here doesn't fit since the key is an Integer and not a String. So we should either drop any second level type restriction on extraProperties or change the example. I vote for the former...

Oops you are right. I was looking at the third level here. Please ignore my comment above.

oh well i added an example anyway! I think it is ok to be extra verbose for now and then once we have some experience working with this we can make some changes.

arose · 2018-10-17T19:01:01Z

lgtm, thanks @danpf! @gtauriello ok too? If so I will merge it.

spec.md

* New fields as in rcsb/mmtf#36 * msgpack zone kept in StructureData to deal with extra data * Updated MapDecoder to init from maps as well * Extended tests and copy, =, ==, != functions for StructureData

danpf added 4 commits August 31, 2018 08:06

update

1c5349e

Updated spec

271a6d2

More conventions

1d6f0de

Fix missing list

36786df

arose reviewed Aug 31, 2018

View reviewed changes

gtauriello reviewed Sep 3, 2018

View reviewed changes

danpf added 2 commits September 14, 2018 09:28

Make parts more clear

17615fb

Convention->reserved

bbc8b7a

danpf added 5 commits October 15, 2018 08:31

more specific properties

0493ad6

fix table formatting

477df35

little note in Extra Data stuff

22d0e47

merge with master

c1a50e0

Added note about reserved fields

12177ef

gtauriello reviewed Oct 15, 2018

View reviewed changes

danpf added 3 commits October 15, 2018 11:59

Try python formatting, and remove structureProperties

d89656e

specify no restrictions on extraProperties second level

2a31ce9

fix spacing

fa85b15

speleo3 reviewed Oct 17, 2018

View reviewed changes

spec.md Outdated Show resolved Hide resolved

fix missing space

98ecbf9

arose merged commit 1945970 into rcsb:master Oct 22, 2018

This was referenced Oct 22, 2018

Remark/comments field #32

Closed

Support for extraData field + test rcsb/mmtf-cpp#15

Merged

gtauriello mentioned this pull request Apr 16, 2019

Where in mmtf is HELIX, SHEET, FORMUL information that is available in the pdb files? rcsb/mmtf-cpp#28

Closed


		Type: [Map](#types)

		Description: A field meant to store information on specific bonds. Must have the same length as [numBonds](#numbonds).


		Description: A field meant to store information on specific bonds. Must have the same length as [numBonds](#numbonds).

		Convention key:value pairs


		Example

		```

Conversation

danpf commented Aug 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danpf Sep 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arose commented Oct 13, 2018

Uh oh!

gtauriello commented Oct 15, 2018

Uh oh!

danpf commented Oct 15, 2018

Uh oh!

gtauriello commented Oct 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arose commented Oct 17, 2018

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

danpf commented Aug 31, 2018 •

edited

Loading

danpf Sep 14, 2018 •

edited

Loading

gtauriello commented Oct 15, 2018 •

edited

Loading