improved plot1d function #1037
improved plot1d function #1037
Conversation
📝 WalkthroughWalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PatchHierarchy
participant Patch
participant Matplotlib
User->>PatchHierarchy: plot1d(**kwargs)
PatchHierarchy->>PatchHierarchy: Determine quantity, levels, time
PatchHierarchy->>Patch: Extract data, coordinates (handle ghost cells)
PatchHierarchy->>Matplotlib: Plot each patch's data with style params
alt plot_patches enabled
PatchHierarchy->>Matplotlib: Draw dashed lines at patch boundaries
end
alt highlight_patches enabled
PatchHierarchy->>Matplotlib: Highlight patch extents with rectangles
end
PatchHierarchy->>Matplotlib: Set labels, limits, grid, legend, title
alt filename provided
PatchHierarchy->>Matplotlib: Save figure
end
PatchHierarchy-->>User: Return figure, axis
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Pylint (3.3.7)pyphare/pyphare/pharesee/hierarchy/hierarchy.py✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
pyphare/pyphare/pharesee/hierarchy/hierarchy.py (2)
446-446: Rename unused loop variable.The loop variable
lvlis not used within the loop body.-for lvl_nbr, lvl in self.levels(time).items(): +for lvl_nbr, _ in self.levels(time).items():
461-461: Fix whitespace before colon.Remove the extra space before the colon to comply with PEP 8.
- x = np.copy(x[nbrGhosts[0] : -nbrGhosts[0]]) + x = np.copy(x[nbrGhosts[0]:-nbrGhosts[0]])
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pyphare/pyphare/pharesee/hierarchy/hierarchy.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
pyphare/pyphare/pharesee/hierarchy/hierarchy.py (1)
Learnt from: PhilipDeegan
PR: PHAREHUB/PHARE#910
File: pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py:7-7
Timestamp: 2024-10-18T13:23:32.074Z
Learning: In the `pyphare.pharesee.hierarchy` module, importing `PatchHierarchy` and `format_timestamp` from `hierarchy.py` into `hierarchy_utils.py` is acceptable as long as `hierarchy.py` does not import `hierarchy_utils.py`, thereby avoiding a cyclic import.
🧬 Code Graph Analysis (1)
pyphare/pyphare/pharesee/hierarchy/hierarchy.py (4)
pyphare/pyphare/pharesee/hierarchy/patchdata.py (1)
x(37-46)pyphare/pyphare/core/box.py (3)
copy(68-69)copy(138-142)shape(32-34)pyphare/pyphare/pharesee/hierarchy/patch.py (1)
copy(43-47)tests/simulator/test_run.py (1)
plot(165-193)
🪛 Ruff (0.11.9)
pyphare/pyphare/pharesee/hierarchy/hierarchy.py
407-407: mpl_toolkits.axes_grid1.make_axes_locatable imported but unused
Remove unused import: mpl_toolkits.axes_grid1.make_axes_locatable
(F401)
446-446: Loop control variable lvl not used within loop body
Rename unused lvl to _lvl
(B007)
🪛 Flake8 (7.2.0)
pyphare/pyphare/pharesee/hierarchy/hierarchy.py
[error] 407-407: 'mpl_toolkits.axes_grid1.make_axes_locatable' imported but unused
(F401)
[error] 461-461: whitespace before ':'
(E203)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: build (ubuntu-latest, gcc)
- GitHub Check: build (ubuntu-latest, clang)
- GitHub Check: build (macos-13)
- GitHub Check: Analyze (python)
- GitHub Check: build (macos-14)
- GitHub Check: Analyze (cpp)
🔇 Additional comments (2)
pyphare/pyphare/pharesee/hierarchy/hierarchy.py (2)
472-502: Well-implemented patch visualization features!The optional patch boundary lines and highlight rectangles provide excellent visual debugging capabilities for AMR hierarchies.
504-525: Excellent improvements to plot configuration and usability!The addition of:
- Smart y-axis limits with 5% padding
- Optional grid display
- Configurable DPI for file saving
- Return of figure and axis objects for further customization
These changes significantly enhance the method's flexibility and usability.
| qty = kwargs.get("qty", None) | ||
| time = kwargs.get("time", self.times()[0]) | ||
| from matplotlib.patches import Rectangle | ||
| from mpl_toolkits.axes_grid1 import make_axes_locatable |
There was a problem hiding this comment.
Remove unused import.
The make_axes_locatable import is not used in this method.
from matplotlib.patches import Rectangle
-from mpl_toolkits.axes_grid1 import make_axes_locatable📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from mpl_toolkits.axes_grid1 import make_axes_locatable | |
| from matplotlib.patches import Rectangle |
🧰 Tools
🪛 Ruff (0.11.9)
407-407: mpl_toolkits.axes_grid1.make_axes_locatable imported but unused
Remove unused import: mpl_toolkits.axes_grid1.make_axes_locatable
(F401)
🪛 Flake8 (7.2.0)
[error] 407-407: 'mpl_toolkits.axes_grid1.make_axes_locatable' imported but unused
(F401)
🤖 Prompt for AI Agents
In pyphare/pyphare/pharesee/hierarchy/hierarchy.py at line 407, remove the
import statement for make_axes_locatable from mpl_toolkits.axes_grid1 since it
is not used anywhere in the method or file, eliminating unnecessary imports.
|
Is it possible to try having a contiguous line plot for contiguous patches. Thinking about it quickly, I suggest trying first to loop through patches and count the total number of nodes contained in patchdatas whose ghost boxes overlap, making an array of that size and copy associated patchdata datatasets into it, and the plot those collections of arrays. |
2 participants
No description provided.