-
Notifications
You must be signed in to change notification settings - Fork 19
Update intermediate files notebook #670
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
Conversation
Ignore line length errors (E501) in `rsmtool.utils.constants.py`.
Add a new dictionary to `utils/constants.py` that contains custom descriptions for the intermediate files generated by `rsmtool` and `rsmsummarize`. We don't add an entry for `rsmeval` since the files are a strict subset of `rsmtool`'s files. We handle subgroups by setting a "ZZZ" placeholder which we can replace with the subgroup name when processing the description.
- Update `rsmtool.utils.notebook.show_files()` to take a context parameter and use that to look up the custom descriptions added to `rsmtool.utils.constants`. - Update `rsmtool.utils.notebook.get_files_as_html()` to use the custom descriptions when available but fail over to using the filename components if not. Also, produce a table rather than a bulleted list as before with rows still sorted by filenames.
Update `rsmtool` and `rsmsummarize` intermediate notebooks to (a) call `show_files()` with the context as a parameter and (b) update the introductory text to be more useful.
- Update `reporter.py` to pass the context as an environment variable when generating `rsmsummarize` reports. - Update the summary header notebook to load this environment variable and make it available to other notebooks.
|
Hello @desilinguist! Thanks for updating this PR.
Comment last updated at 2024-01-16 15:42:02 UTC |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #670 +/- ##
==========================================
+ Coverage 95.58% 95.63% +0.05%
==========================================
Files 32 32
Lines 4503 4515 +12
==========================================
+ Hits 4304 4318 +14
+ Misses 199 197 -2 ☔ View full report in Codecov by Sentry. |
|
Hold off on reviewing, working to improve the test coverage. |
Refactor the tests for `get_files_as_html()` to be more comprehensive and to reduce duplication.
1dcbc1c to
9f4e1f1
Compare
damien2012eng
left a comment
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.
Works great! I ran a few test experiments, and I am able to see custom descriptions and download the intermediate files.
- Do not overwrite rsmeval context as rsmtool. - Add explicit rsmeval key to descriptions dictionary. - Set documentation link based on context for rsmtool/rsmeval. - Update summary notebook to link to rsmsummarize specific link. - Update test to use rsmeval context directly
Also fix minor typo.
|
@tamarl08 I have addressed your comments. Please take a look. @damien2012eng can you please re-run the experiments you ran too to make sure? Thanks! |
|
Pretty cool! It now can direct to the corresponding link. |
|
Looks great! |
This PR updates the intermediate files notebook to be more readable and closes #177.
Changes
utils/constants.pythat contains custom descriptions for the intermediate files generated byrsmtoolandrsmsummarize. We don't add an entry forrsmevalsince the files are a strict subset ofrsmtool's files. We handle subgroups by setting a "ZZZ" placeholder which we can replace with the subgroup name when processing the description.rsmtool.utils.notebook.show_files()to take a context parameter and use that to look up the custom descriptions added torsmtool.utils.constants.rsmtool.utils.notebook.get_files_as_html()to use the custom descriptions when available but fail over to using the filename components if not. Also, produce a table rather than a bulleted list as before with rows still sorted by filenames.rsmtoolandrsmsummarizeintermediate file notebooks to (a) callshow_files()with the context as a parameter and (b) update the introductory text to be more useful.reporter.pyto pass the context as an environment variable when generatingrsmsummarizereports.get_files_as_html()to use tables as expected output.rsmtool.utils.constants.To review
rsmtool,rsmeval, andrsmsummarizeexperiments from the test data and check that the intermediate file section in the notebook is using the custom descriptions and that the links work. Make sure to include experiments that have subgroups, use length, have a second human score, usetsv/xlsxoutput files, etc.