-
Notifications
You must be signed in to change notification settings - Fork 5
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 the ability to disable post-processing for filterable modules #58
Conversation
c3ab905
to
a14790d
Compare
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.
LGTM! This touches a lot of code that I'm not super familiar with, so if you have any uncertainty it might be worth validating with @Dref360, but otherwise I think it makes sense and it's great functionality!
Maybe we should add something to the changelog for this? |
Co-authored-by: Lindsay Brin <lindsay.brin@gmail.com>
…/azimuth into ggm/disabling-post-processing
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.
LGTM, some minor comments
@Dref360 can you review my latest commit (not the merge) since I made a few additional changes? |
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.
LGTM
@@ -105,6 +105,7 @@ def get_utterances( | |||
dataset_split_manager: DatasetSplitManager = Depends(get_dataset_split_manager), | |||
pipeline_index: Optional[int] = Depends(query_pipeline_index), | |||
pagination: Optional[PaginationParams] = Depends(get_pagination), | |||
without_postprocessing: bool = False, |
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.
Sorry, I am late to the party, I didn't realize that was getting merged.
Everywhere this query parameter is defined, it should have an alias to make it camelCase, like the others, for example smartTags
, dataActions
, or confidenceMin
/Max
).
@nandhinibsn, could you add those in your PR?
without_postprocessing: bool = False, | |
without_postprocessing: bool = Query( | |
False, title="Without Postprocessing", alias="withoutPostprocessing" | |
), |
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 agree. I can add those alias on the UI.
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.
Oh so sorry, thanks! I should have tagged you in that PR.
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.
No-no-no, no need to be sorry. One step at a time. 👍
Description:
Add a module option to disable post-processing for all filterable modules. This will allow a new toggle in the control panel to see metrics/top words/confusion matrix/histogram without post-processing.
Checklist:
You should check all boxes before the PR is ready. If a box does not apply, check it to acknowledge
it.
ran
pre-commit run --all-files
at the end.Run
cd webapp && yarn types
while the back-end is running.our users.
README
files and our wiki for any big design decisions, if relevant.