-
Notifications
You must be signed in to change notification settings - Fork 0
Style and format codebase #131
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
alternatively we could use a meaningful default
… style of .py files)
- leave out most multiline string for now (which need dedenting) - one whitespace update in tests
- sometime indent and dedent has to be used - one typo (altair plot, not plotly plot corrected) - new line before python code block in qmd files
- maybe 'Verschlimmbessern': footer can be hard to read (pdf)
- needed to use escaping of {} using unicode:
\u007b is { and \u007d is }
- 🐛 jupyter used relative import -> changed to URL link of LOGO
- unify MONA mentioning (pdf)
lazy logging is recommended (only a warning): https://pylint.readthedocs.io/en/latest/user_guide/messages/warning/logging-fstring-interpolation.html
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.
Pull Request Overview
This pull request makes widespread style and formatting improvements across the codebase, including updates to logging message formatting (switching from f‐strings to % formatting), use of textwrap for dedenting multiline strings, and minor refactorings to exception handling.
- Updated log messages for consistency and better readability
- Reformatted multiline strings using textwrap.dedent and adjusted error chaining
- Minor adjustments in variable naming and documentation for clarity
Reviewed Changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/report_examples/…/quarto_report/*.qmd | Formatting improvements in Quarto report files with dedented multiline strings and updated iframe HTML markup. |
| tests/report_examples/…/streamlit_report/sections/*.py | Consistent reformatting of Streamlit sections using multiline dedent and updated logging message style. |
| src/vuegen/utils/init.py | Updated exception raising and logging; switched to chaining exceptions via “from e”. |
| src/vuegen/constants.py | Added project constants; note the use of curly braces in the GITHUB_ORG_URL_BRACKETS value. |
| Other source files | Similar reformatting and logging message updates across various modules (report_generator, report.py, etc.) for improved maintainability. |
Comments suppressed due to low confidence (1)
src/vuegen/constants.py:5
- Verify that the inclusion of curly braces in GITHUB_ORG_URL_BRACKETS is intentional; if not, update the string to match the expected URL format.
GITHUB_ORG_URL_BRACKETS = "{https://github.com/Multiomics-Analytics-Group}"
sayalaruano
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.
The code looks better with the updates, all seems good, just committed a small fix for loading images from a URL in streamlit reports
…e config update config: - fixed line length to 88 as default - ensure line lenght is checked in tests
|
I added the line length configuration to be 88 characters (black's default) and to check bugbear and other things using ruff.
|
- dedent should be called only once - each block needs to be on the level of where it is supposed to be used (in the multiline f-string)
Added updates to streamlit and quarto qmd files which were used for testing. But each commit should show the respective changes going through them: pull/131/commits
exc_info=Truein case an error is loggedfrom einstead of pasting it into a new error, see here