Bugfix: slurp docs parents column
#534
Conversation
- Delete: unused import - Add: todo comments
- Bug fix: An issue where a | in the parents column would cause the markdown table to be rendered improperly. For all tables other than ordo, this would result in only 1 of the parents showing. For ordo, it would result in one of the parents populating into the subset column, instead of showing the actual subset.
joeflack4
added
bug
| @@ -154,6 +154,8 @@ def slurp_docs(): | |||
| ontology_name = os.path.basename(path).replace(FILENAME_GLOB_PATTERN[1:], '') | |||
| ontology_page_relpath = f'./migrate_{ontology_name.lower()}.md' | |||
| df = pd.read_csv(path, sep='\t').fillna('') | |||
| # Fix issue where the | in the `parents` column is causing table rendering issues | |||
| df['parents'] = df['parents'].apply(lambda x: x.replace('|', ',')) | |||
There was a problem hiding this comment.
The bug fix
@twhetzel I ran this successfully w/out error. Let me know when you've reviewed, and I can run the full build for this per protocol.
There was a problem hiding this comment.
@twhetzel Forgot this was still open.
I have a comment saying:
# Fix issue where the | in the `parents` column is causing table rendering issues
To clarify, there was a jaggedness to the table that was being caused by this problem:
- Markdown tables use
|as a cell delimiter - In ROBOT templates, we usually use
|as a delimiter
This collision was causing the Markdown tables to render multiple cells when there were multiple parents. You can see it if you look in some of the migrate .md files. This PR fixes that.
There was a problem hiding this comment.
Outputs
I didn't commit the .md files that get output into docs/reports/, as the difference generated would be more than just the fix I've implemented. The inputs I have are different than the last time when those were run, so there would be other differences in the tables.
@matentzn @twhetzel I've opted for (a), but let me know your thoughts on showing outputs for this bug fix:
a. Don't commit .md outputs along w/ this PR
b. If either of you do want me to run and commit the outputs, as this is just getting merged into develop, I can do that.
c. If we run the full build, then those outputs will come with it, and be up-to-date.
d. Alternatively, if you want I can attach static files of the outputs to this comment or show a screenshot.
Overview
Fixes an issue where a | in the parents column would cause the markdown table to be rendered improperly. For all tables other than ordo, this would result in only 1 of the parents showing. For ordo as of #531, it would result in one of the parents populating into the subset column, instead of showing the actual subset.
Pre-merge checklist
Documentation
Was the documentation added/updated under
docs/?QC
Was the full pipeline run before submitting this PR using
sh run.sh make build-mondo-ingeston this branch (afterdocker pull obolibrary/odkfull:dev), and no errors occurred?New Packages
Were any new Python packages added?
Were any other non-Python packages added?
PR Review and Conversations Resolved
Has the PR been sufficiently reviewed by at least 1 team member of the Mondo Technical team and all threads resolved?
@souzadevinicius FYI