Skip to content

reformat problematic plotting#187

Merged
kmdalton merged 2 commits intors-station:mainfrom
hkwang:careless_update_plotting
Aug 28, 2025
Merged

reformat problematic plotting#187
kmdalton merged 2 commits intors-station:mainfrom
hkwang:careless_update_plotting

Conversation

@hkwang
Copy link
Contributor

@hkwang hkwang commented Aug 6, 2025

forced the legends above plots for ccpred.py and image_cc.py and changed default height and width of each plot. Added height and width arguments for ccpred.py. Forced legends to the upper left for rsplit.py.

Copy link
Member

@kmdalton kmdalton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good idea, but we should think a bit about what we want to do. There is a danger that as we go down this path of adding workarounds, we end up supporting the entire matplotlib API via the command line parser. I want to avoid that. This is one reason I provide the --show flag so users can tweak the plot interactively using the mpl GUI. Regardless, it's a good idea to have sensible defaults at the CLI if possible. I'm happy to greenlight some solution based on passing image dimensions to the parser. Before I merge this, I would just request that we make the --width and --height arguments consistently implemented across the stats submodule. I think we can do this by adding them to the BaseParser and double checking that plt.figure is called using the parser.height and parser.width in each of the subprograms.

help="Pool all prediction mtz files into a single calculation rather than treating each file individually.",
)

self.add_argument(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If arguments like this are to be added, I would request that they are added in careless/states/parser.py:BaseParser and then propagated to all the stats submodules.

@hkwang
Copy link
Contributor Author

hkwang commented Aug 27, 2025

See commit above, in reply to change request. I'll also push a change to careless-examples reflecting use of the --show flag.

@hkwang
Copy link
Contributor Author

hkwang commented Aug 27, 2025

alright, regarding the previous comment, see rs-station/careless-examples@b7b887d

Copy link
Member

@kmdalton kmdalton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, @hkwang. Thanks for your help!

@kmdalton kmdalton merged commit e8e077c into rs-station:main Aug 28, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants