Uh oh!
There was an error while loading. Please reload this page.
Figure.histogram: Split parameter bar_width into bar_width and bar_offset - #4316
Conversation
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
seisman
commented
Dec 26, 2025
Looks good to me, except for one minor comment above. Are there any changes you'd like to make? |
Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
I think the image comparison for the new test was missing. I tried to create the baseline image and pushed the DVC file to GitHub and the PNG file to DagsHub. But it looks like something did not work correctly. I remember that there should be a GitHub comment showing a summary of the modified or newly added baseline images. But this is currently not the case. |
| return fig | ||
| @pytest.mark.mpl_image_compare(filename="test_histogram_barwidth_baroffset.png") |
There was a problem hiding this comment.
This is the default file name, so no need to specify it explicitly.
| @pytest.mark.mpl_image_compare(filename="test_histogram_barwidth_baroffset.png") | |
| @pytest.mark.mpl_image_compare |
There was a problem hiding this comment.
Actually I feel we don't need to add this test, because there is no complex code logic for these two parameters.
seisman
commented
Dec 27, 2025
Did you run |
Description of proposed changes
This PR splits the parameter
bar_widthinto two parametersbar_widthandbar_offsetto make the codes more Pythonic.Related to #4279 (comment). Extends #4279
Preview:
Guidelines
Slash Commands
You can write slash commands (
/command) in the first line of a comment to performspecific operations. Supported slash command is:
/format: automatically format and lint the code