-
Notifications
You must be signed in to change notification settings - Fork 7
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
Missing value functionality for assessment tool groups + related changes #159
Conversation
- New component `annot-tool-group` allows selecting missing values for columns in tool groups and keeps track of corresponding subject availability - `annot-grouped-values` component renamed to `annot-tool-group` component - `annot-tab` component now supports requirements for new component `annot-tool-group` - `annot-vocabulary-column` component is removed since it has been replaced by the new `annot-tool-group` - `annot-vocabulary-row` is now just named `annot-vocabulary` - Entry criteria to annotation page from categorization page now includes the selection of one (and only one) subject - Instructions for subject-column categorization and tool grouping now appear near 'next page' button on categorization if criteria are unmet - Adjustments for spacing with 'next page' button on categorization page in `categ-tool-group` - New getter `getColumnOfCategory` in store - New `isDataAnnotated` criteria in store includes if new columns have been added to the annotated data table. This is from the new 'available' columns added to the table by the new `annot-tool-group` component when determining subject availability for each tool group - transformHeuristics temporarily commented out - General cleanup: added comments, removing commented out code, removing unused values/props, etc.
My first guess would be the component naming in |
conditional instructions fix on categorization page
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for this PR @jarmoza. (now that I got the right branch ...) I can confirm all works as expected. Very excited for this, that puts us in reaching distance of having a fully functioning prototype of the annotator. Huge milestone (we have to celebrate this).
I made some minor comments (mostly about default props) that would be good to put in so we don't forget, but otherwise this looks good to go!
I also made two more general comments about the subject-availability workflow. At the moment it is quite complex (in terms of its logic flow, inside and outside of the component). Maybe when we are rethinking the organization of the store would be a good time to revisit this and see if we could put the availability logic closer to where the data are stored.
idField: { default: () => {}, required: true, type: String }, | ||
relevantColumns: { default: () => [], required: true, type: Array }, | ||
uniqueValues: { default: () => {}, required: true, type: Object }, | ||
uniqueValuesToSubjectMap: { default: () => {}, required: true, type: Object } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for the required props, we don't have to define defaults here. Doesn't hurt, but makes you think when this would become relevant - and I think it can't, right? type and required should be enough
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me. Re: props, there's definitely a lack of standards. A few questions come up currently. This is one question. (And I agree with you - wasn't sure about the need for a default key.)
Another question is whether all props being passed to a child component that can be multiple components (<component :is="component-name"
...) need to be shown in the child component if they are unused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, also don't know about that second one. Could make it an issue with low priority to investigate once we have several of these collected?
// // } | ||
// return true; | ||
// }, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
horray! 🎉 cleanup
|
||
const tableArray = []; | ||
|
||
// In 'row' mode create table entries for each value in the relevant columns |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm usually all for throwing way things but in this case I think keeping a high level comment about what the method does would be helpful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll place this back and adapt the comment for the current implementation.
functionality for vocabulary and tool group components
A new component
annot-tool-group
allows for the proper assessment of missing values within assessment tool columns that have been grouped together on the categorization page.true
/false
value that reflects the combined boolean value of each of the subject's column availability (e.g. if any column values for a subject are missing, it is 'unavailable' for that tool group).annot-tab
creates and passes along a new computed valueuniqueValuesToSubjectMap
to facilitate the connection between subjects and unique values forannot-tool-group
.isDataAnnotated
inindex.js
)annot-vocabulary-row
is now justannot-vocabulary
andannot-vocabulary-column
has been deleted since its functionality has been replaced by the newannot-tool-group
component.