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

Implement missing values discrete #132

Merged
merged 22 commits into from
May 9, 2022
Merged

Conversation

surchs
Copy link
Contributor

@surchs surchs commented May 6, 2022

This PR implements the interface and logic to declare missing values in the discrete-values component (atm used for the sex category).

Criteria from the issue were:
Accept when:

  • I have an interface element that can be used to designate a value as missing
  • using the interface element causes this value to be removed from the list of values to be annotated
  • using the interface element also causes the value to be stored in the global store in the list/object for "missing values"
  • the value will then be shown by the corresponding missing value component
  • when the value is removed from the list of missing values via the missing-value-component, it shows up again in my list of values to annotate
  • when that happens, the mapping of the value should be empty and the annotation status should be consider incomplete
  • this will also be reflected in computing the annotation status of the category
  • when the annotation is "saved" with missing values labeled, these values will be annotated as "missing" or something consistent like that.

The main addition here was making the state check for the "save annotation" button a computed property and turn all of its dependencies (e.g. this.valueMapping) reactive.

Resolves #120

surchs added 18 commits May 4, 2022 11:57
- discrete values component now has an extra UI element (red button) for each unique value that can label this value as "missing"
- so far it doesn't do anything beyond being visible
- discrete value component can now emit an event when the "missing value" button is clicked to ask that the corresponding value be labeled as missing
- the event is forwarded up to the annotation page
- on the annotation page, a new method addMissingValue handles the event and saves the information to the global store
- only display unique values to be annotated that are not missing values
- designating a value as missing now removes it from the list of values to be annotated and will
- annotation status will be true only when all unique values are mapped to a value that is not null and
- if any non-mapped values are labeled as missing.
A number of things have changed in order to have a reactive annotation state check that can handle these cases:
- an unannotated value has been declared missing
- a unique value has been annotated

Changes:
- the valueMapper initialization has to happen during the "created" lifecycle stage so that we can use the valueMapping property in a computed prop
- the button status is now computed property
- this change allows us to scrap the previous watcher hack for the relevantColumn Property
- in order to merge new annotations with the valueMapper property while not breaking reactivity, we have to do a deep merging of the objects. This is now done inside the updateMapping method
- discrete values component now has an extra UI element (red button) for each unique value that can label this value as "missing"
- so far it doesn't do anything beyond being visible
- discrete value component can now emit an event when the "missing value" button is clicked to ask that the corresponding value be labeled as missing
- the event is forwarded up to the annotation page
- on the annotation page, a new method addMissingValue handles the event and saves the information to the global store
- only display unique values to be annotated that are not missing values
- designating a value as missing now removes it from the list of values to be annotated and will
- annotation status will be true only when all unique values are mapped to a value that is not null and
- if any non-mapped values are labeled as missing.
A number of things have changed in order to have a reactive annotation state check that can handle these cases:
- an unannotated value has been declared missing
- a unique value has been annotated

Changes:
- the valueMapper initialization has to happen during the "created" lifecycle stage so that we can use the valueMapping property in a computed prop
- the button status is now computed property
- this change allows us to scrap the previous watcher hack for the relevantColumn Property
- in order to merge new annotations with the valueMapper property while not breaking reactivity, we have to do a deep merging of the objects. This is now done inside the updateMapping method
- discrete values component now has an extra UI element (red button) for each unique value that can label this value as "missing"
- so far it doesn't do anything beyond being visible
- only display unique values to be annotated that are not missing values
- designating a value as missing now removes it from the list of values to be annotated and will
A number of things have changed in order to have a reactive annotation state check that can handle these cases:
- an unannotated value has been declared missing
- a unique value has been annotated

Changes:
- the valueMapper initialization has to happen during the "created" lifecycle stage so that we can use the valueMapping property in a computed prop
- the button status is now computed property
- this change allows us to scrap the previous watcher hack for the relevantColumn Property
- in order to merge new annotations with the valueMapper property while not breaking reactivity, we have to do a deep merging of the objects. This is now done inside the updateMapping method
@surchs surchs requested a review from jarmoza May 6, 2022 16:50
@surchs
Copy link
Contributor Author

surchs commented May 6, 2022

I did a bit of a funny with my rebasing (picked master instead of the previous feature branch this was based off of). So for now, ignore the commit history of this branch. I will probably do another rebase to clean this up before we merge it.

Copy link
Contributor

@jarmoza jarmoza left a comment

Choose a reason for hiding this comment

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

A few code cleanup items to attend, and my one question on the multi-level Object.assign. Other than that, I tested and it works as described in the writeup.

components/annot-discrete-choices.vue Show resolved Hide resolved
components/annot-discrete-choices.vue Outdated Show resolved Hide resolved
components/annot-discrete-choices.vue Show resolved Hide resolved
components/annot-discrete-choices.vue Show resolved Hide resolved
pages/annotation.vue Show resolved Hide resolved
pages/annotation.vue Outdated Show resolved Hide resolved
store/index.js Outdated Show resolved Hide resolved
components/annot-discrete-choices.vue Outdated Show resolved Hide resolved
@jarmoza
Copy link
Contributor

jarmoza commented May 9, 2022

You're clear to merge once (and if) you're addressing the commonStrings suggestion.

@surchs surchs merged commit 7c66748 into master May 9, 2022
@surchs surchs deleted the implement-missing-values-discrete branch May 9, 2022 20:58
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.

Add missing value for sex
2 participants