Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add documentation about metadata #520

Merged
merged 6 commits into from
Dec 20, 2024
Merged

add documentation about metadata #520

merged 6 commits into from
Dec 20, 2024

Conversation

M-Kusumgar
Copy link
Contributor

Let me know what your thoughts are on this! Hopefully it is clear.

@M-Kusumgar M-Kusumgar force-pushed the metadata-documentation branch from 87976f2 to 0f3f301 Compare July 29, 2024 11:29
@M-Kusumgar M-Kusumgar changed the base branch from master to metadata-defaults July 29, 2024 11:30

The plot settings controls is an object with keys of the relevant plot name (e.g. `choropleth`) and value with the structure shown in the visualisation. Explanation of keys:

1. `defaultEffect` is the base effect applied to all `settings` (remember these are the values of the `control` dropdown) - more on this later

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the first time you mention the concept of an effect. It's distracting to read this before knowing what an effect is. Also I think that below in the actual 'Effects' section you don't give a generic explanation of what 'effects' are in this context. Somewhere, ideally before you use the concept, I think you need a quick sentence to say what an effect is, e.g. "A 'setting' (a value for a control) may have one or more 'effects', that is, things that should happen in the front-end when the setting is selected. An example of an effect would be that if you make a selection from control 1, control 2 should turn from a single-select input into a multi-select." You can profitably duplicate that sentence into the 'Effects' section too, because people might try to start reading about them there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, I think a quick description of what you mean by an effect would be useful when you introduce controls and settings, so the reader has some idea what these are when they first see them in the diagram.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I've got confused again about how controls, settings, options and effects all fit together here.
Rather than just showing this structure and describing the parts of it, I think it would be really useful to screenshot a relevant part of the UI and work through it, saying which parts of the metadata contributed to what parts of the UI, and how e.g. changing a preset activates new bits of the metadata.

metadata-structure/metadata-structure.md Outdated Show resolved Hide resolved
metadata-structure/metadata-structure.md Outdated Show resolved Hide resolved
metadata-structure/metadata-structure.md Outdated Show resolved Hide resolved
metadata-structure/metadata-structure.md Outdated Show resolved Hide resolved
metadata-structure/metadata-structure.md Outdated Show resolved Hide resolved
Copy link
Contributor

@EmmaLRussell EmmaLRussell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, only got halfway through this before becoming completely confused, and you've already explained this to me at least once!

I really like the thoroughness of the explanation but I feel like starting from the metadata structure and trying to explain the individual bits of it doesn't really help my understanding. I remember when you explained it to me before in terms of the different use cases and why each part of the metadata was required to support each of them, that was really useful. I feel like an explanation which starts from the parts of the UI - ideally as screenshots with arrows showing which bits map to settings, controls, effects etc. in the corresponding metadata example - would make this all seem less abstract than it does currently. Sorry that would be quite a lot of work, but I feel like it would be worth it!

Also, just a few style and clarity suggestions for the first part of text..

metadata-structure/metadata-structure.md Outdated Show resolved Hide resolved
metadata-structure/metadata-structure.md Outdated Show resolved Hide resolved
metadata-structure/metadata-structure.md Outdated Show resolved Hide resolved
metadata-structure/metadata-structure.md Outdated Show resolved Hide resolved
metadata-structure/metadata-structure.md Outdated Show resolved Hide resolved
metadata-structure/metadata-structure.md Outdated Show resolved Hide resolved
metadata-structure/metadata-structure.md Outdated Show resolved Hide resolved

The plot settings controls is an object with keys of the relevant plot name (e.g. `choropleth`) and value with the structure shown in the visualisation. Explanation of keys:

1. `defaultEffect` is the base effect applied to all `settings` (remember these are the values of the `control` dropdown) - more on this later
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, I think a quick description of what you mean by an effect would be useful when you introduce controls and settings, so the reader has some idea what these are when they first see them in the diagram.


The plot settings controls is an object with keys of the relevant plot name (e.g. `choropleth`) and value with the structure shown in the visualisation. Explanation of keys:

1. `defaultEffect` is the base effect applied to all `settings` (remember these are the values of the `control` dropdown) - more on this later
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...ok, so the default effect is like the baseline and the other effects describe how they deviate from the baseline?


The plot settings controls is an object with keys of the relevant plot name (e.g. `choropleth`) and value with the structure shown in the visualisation. Explanation of keys:

1. `defaultEffect` is the base effect applied to all `settings` (remember these are the values of the `control` dropdown) - more on this later
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I've got confused again about how controls, settings, options and effects all fit together here.
Rather than just showing this structure and describing the parts of it, I think it would be really useful to screenshot a relevant part of the UI and work through it, saying which parts of the metadata contributed to what parts of the UI, and how e.g. changing a preset activates new bits of the metadata.

Copy link
Collaborator

@r-ash r-ash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thanks for writing this up. Couple of small comments I left, and a couple of other things.

  1. Could you make this into a vignette, we can then put it onto a docs site in a later PR
  2. Could you expand the bit about default effect, I can't quite remember a couple of things after not looking at this for a while. These get run when the payload is received right to set the initial state? Do they also get run when we change any of the plot controls is that right? In that way if we have an effect in the default and in the plot control then they interact as explained in the "Effects" seciton? As in setFilters will override the default, but setMultiple is additive?

metadata-structure/metadata-structure.md Outdated Show resolved Hide resolved
metadata-structure/metadata-structure.md Outdated Show resolved Hide resolved
metadata-structure/metadata-structure.md Outdated Show resolved Hide resolved
1. `setFilters` is how the effect specifies what `filters` the user sees from the `filterTypes` The frontend aggregates this by replacing it. So if two `settings` set filters, the last `setFilters` in the for loop would be used when going through all the `settings`. Keys:
1. `filterId` is the `id` key in a `filterTypes`element (`a` in the visualisation)
1. `label` is the title of the `filter` displayed to the user in the UI
1. `stateFilterId` is the id the state uses to identify this filter. This is distinct to the `filterId` because in the bubble plot we have two indicators filters that are the same filter type (so have the same `options` and the same `column_id`) however they are distinct because one controls the color of the bubbles and one controls the size of the bubbles so while both their `filterId`s are `indicator`, the `stateFilterId`s for both are different, `colorIndicator` and `sizeIndicator`. For most cases this can be set as the same thing as the `filterId`. All other effects will now identify filters by `stateFilterId` as these uniquely correspond to what the user sees
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the take home from this is that stateFilterId should be used as the primary key for the filter? So all effects reference it via its stateFilterId. The fitlerId should only be used to actually filter the data itself and to identify the filterType?

@r-ash r-ash changed the base branch from metadata-defaults to tmp-epic-plot-cleanup September 6, 2024 12:33
@r-ash r-ash force-pushed the metadata-documentation branch from 0f3f301 to 77c4364 Compare December 19, 2024 18:31
@r-ash r-ash changed the base branch from tmp-epic-plot-cleanup to main December 19, 2024 18:39
vignettes/metadata.Rmd Outdated Show resolved Hide resolved
Co-authored-by: M-Kusumgar <98405247+M-Kusumgar@users.noreply.github.com>
@r-ash r-ash merged commit 6b0b424 into main Dec 20, 2024
8 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants