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 component #123

Merged
merged 5 commits into from
May 5, 2022

Conversation

surchs
Copy link
Contributor

@surchs surchs commented May 4, 2022

This PR implements the version of the missing value component we have discussed in #58. The component does not detect or create missing values, it only does two things:

  • for the currently relevant column names, show the missing values currently kept in the global store
  • provide a UI element (button) to declare any of these values as "not missing". this will trigger an event chain and update the global store by removing this value from the array of missing values of its columnName

Writing to missingColumnValues in the store via the setMissingColumnValues mutation was not reactive, because Vue doesn't track indexed changes to Objects (see https://v2.vuejs.org/v2/guide/reactivity.html). I have replaced the store logic with an Object.assign statement. This will continue to work with the partial updates we are receiving from the annotation tab,

Example:

let state_missingColumnValues = {
        "sex": ["na", "999"],
        "diagnosis": ["AD", "none", "dunno"] // AD is not a missing value, so we remove it
};
const new_missing_values = {"diagnosis": ["none", "dunno"]};
state_missingColumnValues = Object.assign({}, state_missingColumnValues, new_missing_values);
console.log(state_missingColumnValues);

// { sex: [ 'na', '999' ], diagnosis: [ 'none', 'dunno' ] }

Another thing I did here is to give the missing Value Component direct access to the store state. This is different from what we do for the other components and what was previously done, where we give only the pages (e.g. annotation) access to the store and then provide/inject or prop-chain the information down to the components. Two reasons why I did it this way here:

  • the component already has state access for the getter (valueDescription)
  • none of the other components needs access to this information so provide / inject seems a bit overkill. similarly so would be props edit: I just realized that this is not true. Other components will need this as well. So yeah, maybe we should use provide / inject
  • it is easier to see what's going on while inside the component (this is a thing from the state).

But open to change to props or provide / inject.

Sidenotes:

  • I'm not sure why I'm still getting white-space diffs here, I thought I had solved that by running eslint beforehand. Sorry about that.
  • because this gets implemented before we have a way to "create" missing values, and because the missing values component no longer makes its own guesses, I stuck some temporary event emitter in the annotation page to create some missing values to debug with (targeted to our example.tsv file). I'll remove these once we have a way to add missing values!

Resolves #58

surchs added 4 commits May 3, 2022 17:39
- only using test data
- nothing connected yet
- assignment to missingValueColums in store is reactive now
- component reactively shows global store
- filters by relevantColumns
- has event emitter to "remove value" and have this be stored in store
- card title bar fills full width
@surchs surchs requested a review from jarmoza May 4, 2022 15:06
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.

Some minor formatting changes. We have to check why our linter has not picked up on missing semicolons.

Feel free to merge after you fix the formatting issues.

components/annot-missing-values.vue Show resolved Hide resolved
components/annot-missing-values.vue Show resolved Hide resolved
pages/annotation.vue Outdated Show resolved Hide resolved
store/index.js Show resolved Hide resolved
@surchs surchs merged commit 95bbc0b into master May 5, 2022
@surchs surchs deleted the implement-missing-values-component branch May 10, 2022 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Create a component to show missing data
2 participants