-
Notifications
You must be signed in to change notification settings - Fork 0
13 create a gallery #15
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
Reviewer's GuideIntroduces a full “gallery” of Jupyter‐based tutorials integrated into Sphinx, updates CI to auto‐build and publish HTML docs, refactors a couple of plotting interfaces, and cleans up the existing tutorial notebooks for consistency. Class diagram for updated plotting interfacesclassDiagram
class Canvas {
+plot_matplotlib(show=True, savefig=False, layers=None, usetex=False)
}
class TikzFigure {
+plot_matplotlib(ax, layers=None)
}
class LinePlot {
+add_node(x, y, label=None, content="", layer=0, **kwargs)
}
Canvas --> TikzFigure : add_tikzfigure()
Canvas --> LinePlot : add_subplot()
TikzFigure <|-- LinePlot
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @max-models - I've reviewed your changes - here's some feedback:
- It looks like you left debug
printstatements incanvas.plot_matplotlibandline_plot.add_node—please remove those so the API is clean. - This PR mixes huge notebook-output stripping with API changes and CI additions—consider splitting notebook cleanup, code refactors (like the
plot_matplotlibsignature change), and docs/CI updates into smaller focused PRs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- It looks like you left debug `print` statements in `canvas.plot_matplotlib` and `line_plot.add_node`—please remove those so the API is clean.
- This PR mixes huge notebook-output stripping with API changes and CI additions—consider splitting notebook cleanup, code refactors (like the `plot_matplotlib` signature change), and docs/CI updates into smaller focused PRs.
## Individual Comments
### Comment 1
<location> `src/maxplotlib/canvas/canvas.py:208` </location>
<code_context>
for (row, col), subplot in self.subplots.items():
ax = axes[row][col]
+ print(f"{subplot = }")
subplot.plot_matplotlib(ax, layers=layers)
# ax.set_title(f"Subplot ({row}, {col})")
</code_context>
<issue_to_address>
Debug print statement left in production code.
Remove the print statement or replace it with appropriate logging if necessary.
</issue_to_address>
### Comment 2
<location> `src/maxplotlib/subfigure/line_plot.py:250` </location>
<code_context>
if layer in self.layers:
self.layers[layer].add(node)
else:
+ print(f"{self.layers = } {layer = }")
self.layers[layer] = Tikzlayer(layer)
self.layers[layer].add(node)
</code_context>
<issue_to_address>
Debug print statement should be removed.
Consider removing this print statement or replacing it with a logger at the appropriate log level.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Published docs to https://max-models.github.io/maxplotlib/
For now, the tutorials are simply generated