Skip to content

Split plots builtins by base class - #1550

Merged
bdlucas1 merged 3 commits into
Mathics3:masterfrom
bdlucas1:split-plots-builtins-by-base-class
Dec 21, 2025
Merged

bdlucas1 merged 3 commits into
Mathics3:masterfrom
bdlucas1:split-plots-builtins-by-base-class

Conversation

@bdlucas1

Copy link
Copy Markdown
Collaborator

This is a preliminary code rearrangement to make subsequent refactoring easier to do and easier to review. Plotting functions from plot.py that shared common base classes were pulled out into separate files by base class. Mostly a mechanical cut&paste operation, with a little manual cleanup to move variables defined at the top of the file.

@mmatera mmatera left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this is a good idea. My only observation is that the usual pattern for this seems to be to create a module mathics.builtin.drawing.plot, and then include inside plot.py->plot_base.py and the other files inside.

@bdlucas1

Copy link
Copy Markdown
Collaborator Author

Maybe that's where we'll end up, but at this point there is still some significant function in plot.py so creating a plot module would hide that significant function away in init.py which I think would be confusing (at least, it would confuse me). At this point I'd rather focus on what's going on in plot_plot.py, so maybe we can consider this a step along the way.

@mmatera

mmatera commented Dec 21, 2025

Copy link
Copy Markdown
Contributor

Maybe that's where we'll end up, but at this point there is still some significant function in plot.py so creating a plot module would hide that significant function away in init.py which I think would be confusing (at least, it would confuse me). At this point I'd rather focus on what's going on in plot_plot.py, so maybe we can consider this a step along the way.

In that case, LGTM

@bdlucas1
bdlucas1 merged commit 342f0ab into Mathics3:master Dec 21, 2025
17 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