-
Notifications
You must be signed in to change notification settings - Fork 3
Addressing missing fonts in Linux #56
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
…ase of fallback to a valid font
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 addresses missing fonts in Linux by adding font family customization options to volcano plots and bubble maps, and suppressing matplotlib font manager warning messages for missing system fonts.
- Adds a
font_familyparameter to plotting functions allowing users to specify custom font families - Suppresses matplotlib font_manager warnings by setting the logger level to ERROR when a valid system fallback font exists
- Removes trailing whitespace in several functions
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| src/kinase_library/modules/enrichment.py | Adds logging configuration to suppress font warnings and implements font_family parameter in plot_volcano and plot_bubblemap functions |
| src/kinase_library/enrichment/mea.py | Adds font_family parameter to plot_volcano method and passes it through to the base plotting function; removes trailing whitespace |
| src/kinase_library/enrichment/differential_phosphorylation.py | Adds logging configuration to suppress font warnings and implements font_family parameter in volcano plotting methods; removes trailing whitespace |
| src/kinase_library/enrichment/binary_enrichment.py | Adds font_family parameter to plot_volcano method and passes it through to the base plotting function; removes trailing whitespace |
Comments suppressed due to low confidence (5)
src/kinase_library/modules/enrichment.py:199
- combine_diff_phos_enrichment_results returns tuple of size 2 and tuple of size 3.
def combine_diff_phos_enrichment_results(enrichment_results_dict, enrichment_type='combined', data_type='kl_object',
lff_col_name=None, pval_col_name=None, cont_kins_col_name=None):
src/kinase_library/modules/enrichment.py:507
- Except block directly handles BaseException.
except:
src/kinase_library/enrichment/binary_enrichment.py:609
- Instantiating an exception, but not raising it, has no effect.
ValueError(f'Warning: kins_label_dict has unexpected keys: {set(kins_label_dict) - set(kins_labels.values())}')
src/kinase_library/enrichment/differential_phosphorylation.py:707
- Instantiating an exception, but not raising it, has no effect.
ValueError(f'Warning: kins_label_dict has unexpected keys: {set(kins_label_dict) - set(kins_labels.values())}')
src/kinase_library/enrichment/mea.py:573
- Instantiating an exception, but not raising it, has no effect.
ValueError(f'Warning: kins_label_dict has unexpected keys: {set(kins_label_dict) - set(kins_labels.values())}')
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if font_family: | ||
| mpl.rcParams['font.family'] = font_family |
Copilot
AI
Dec 31, 2025
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.
Modifying global matplotlib rcParams within a function can have unintended side effects. The font_family change persists after the function returns and affects all subsequent matplotlib operations in the same Python session, not just this plot. Consider using a context manager approach to temporarily change the font family. For example, you could use with mpl.rc_context({'font.family': font_family}): to wrap the plotting code, which would restore the original font settings after the plot is complete.
| font_family : string, optional | ||
| Customized font family for the figures. The default is None. |
Copilot
AI
Dec 31, 2025
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 parameter type should be 'str' instead of 'string' to maintain consistency with other parameter documentation in this file. All other string parameters in the codebase use 'str' (e.g., lines 390, 392, 421, 575, etc.).
| font_family : string, optional | ||
| Customized font family for the figures. The default is None. |
Copilot
AI
Dec 31, 2025
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 parameter type should be 'str' instead of 'string' to maintain consistency with other parameter documentation in this file. All other string parameters in the codebase use 'str' (e.g., lines 752, 770, 772, etc.).
| font_family : string, optional | ||
| Customized font family for the figures. The default is None. |
Copilot
AI
Dec 31, 2025
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 parameter type should be 'str' instead of 'string' to maintain consistency with other parameter documentation in this codebase. All other string parameters in the file use 'str' (e.g., lines 527, 531, 533, 538, etc.).
| font_family : string, optional | ||
| Customized font family for the figures. The default is None. |
Copilot
AI
Dec 31, 2025
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 parameter type should be 'str' instead of 'string' to maintain consistency with other parameter documentation in this codebase. All other string parameters in the file use 'str' (e.g., lines 667, 671, 673, 678, etc.).
| font_family : string, optional | ||
| Customized font family for the figures. The default is None. |
Copilot
AI
Dec 31, 2025
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 parameter type should be 'str' instead of 'string' to maintain consistency with other parameter documentation in this codebase. All other string parameters in the file use 'str' (e.g., lines 847, 849, 851, 856, etc.).
| font_family : string, optional | ||
| Customized font family for the figures. The default is None. |
Copilot
AI
Dec 31, 2025
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 parameter type should be 'str' instead of 'string' to maintain consistency with other parameter documentation in this codebase. All other string parameters in the file use 'str' (e.g., lines 555, 559, 561, 566, etc.).
| if font_family: | ||
| mpl.rcParams['font.family'] = font_family |
Copilot
AI
Dec 31, 2025
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.
Modifying global matplotlib rcParams within a function can have unintended side effects. The font_family change persists after the function returns and affects all subsequent matplotlib operations in the same Python session, not just this plot. Consider using a context manager approach to temporarily change the font family. For example, you could use with mpl.rc_context({'font.family': font_family}): to wrap the plotting code, which would restore the original font settings after the plot is complete.
Added an option to customize font family in volcano plots and bubble maps
Suppressed missing fonts messages if system's fallback is a valid font
Closes #54