-
Notifications
You must be signed in to change notification settings - Fork 8
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
[ENH] Implemented Annotate Another Dataset
button
#832
Conversation
Reviewer's Guide by SourceryThis PR implements a new feature to allow users to annotate another dataset by adding a reset functionality and improves the search filtering in dropdown selectors. The changes include adding a new button on the download page, implementing a state reset mechanism, and enhancing the filtering logic for v-select components. Sequence Diagram for State Reset and NavigationsequenceDiagram
actor User
participant DownloadPage
participant Store
participant HomePage
User->>DownloadPage: Click 'Annotate Another Dataset'
DownloadPage->>Store: Call resetState()
Store-->>DownloadPage: State reset
DownloadPage->>HomePage: Navigate to home page
HomePage-->>User: Ready for new annotation
Class Diagram for State Management ChangesclassDiagram
class Store {
+getDefaultState() State
+resetState(p_state)
}
class State {
+annotationCount: int
+columnToToolMap: Map
+resetState()
}
Store --> State
note for Store "Store now uses getDefaultState for initialization and resetState for resetting state."
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
✅ Deploy Preview for neurobagel-annotator ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Hey @rmanaem - I've reviewed your changes - here's some feedback:
Overall Comments:
- Please fill out the 'Changes proposed' section in the PR description, including details about the new Annotate Another Dataset button, dropdown filtering improvements, and rationale for removing integer type support.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
One suggestion, otherwise 🧑🍳
sidenote: The diff on these chained PRs is a bit tricky to read. It works because they are short, but even happier would be either:
- last PR merges into previous branch it chained off of (assuming that is not merged yet)
- every PR is branched off (and targeting)
main
directly.
Checklist
This section is for the PR reviewer
[ENH]
,[FIX]
,[REF]
,[TST]
,[CI]
,[MNT]
,[INF]
,[MODEL]
,[DOC]
) (see our Contributing Guidelines for more info)Closes #XXXX
For new features:
For bug fixes: