formatting python via black #792

PhilipDeegan · 2023-12-10T21:32:44Z

lots of imports cleanup on funcitonal tests so should really run it through the nightly build before merge.

first commit is just black
second commit is import cleanup

Summary by CodeRabbit

New Features
- Introduced a new Search class with methods for fetching and displaying results.
- Added a search bar at the top of the Hero component.
- Added a Search component to the App component.
Enhancements
- Improved search functionality with a user-friendly search bar and result display.
- Added styles for the search bar to enhance visual appeal.
Bug Fixes
- No bug fixes mentioned in the provided information.
Refactor
- No refactoring mentioned in the provided information.
Documentation
- No documentation changes mentioned in the provided information.
Style
- No style changes mentioned in the provided information.
Tests
- No test-related changes mentioned in the provided information.
Chores
- No chore-related changes mentioned in the provided information.

coderabbitai · 2023-12-10T21:33:20Z

Walkthrough

The changes across the pyphare project involve a series of code cleanups, refactoring, and behavior modifications. These include formatting adjustments for readability, removal of redundant code, renaming of methods for consistency, and changes in method implementations. New functions have been added, and exception handling has been improved in some cases. The changes span across various modules, including core data structures, utilities, simulation setup, diagnostics, and testing.

Changes

File(s)	Summary of Changes
`.../core/box.py`, `.../core/gridlayout.py`, `.../core/phare_utilities.py`	Refactored code for improved readability and consistency, including method renaming and formatting adjustments.
`.../cpp/__init__.py`	Modified import statements and added a blank line for better code organization.
`.../data/wrangler.py`	Reformatting of class definition, addition of new methods, and adjustments to existing methods for improved functionality.
`.../pharein/__init__.py`	Renamed variables, reorganized import statements, and added new functions for enhanced functionality.
`.../pharein/diagnostics.py`	Modifications to functions and class methods for improved diagnostics handling.
`.../pharein/electron_model.py`	Reformatting of class methods for better code structure and readability.
`.../pharein/examples/job.py`	Changes in simulation configuration, model parameters, and diagnostics configurations for improved setup.
`.../pharein/global_vars.py`	Minor formatting adjustments in variable declaration.
`.../pharein/init.py`	Modified function to set environment variable and added a comment for better code understanding.
`.../pharein/maxwellian_fluid_model.py`	Reformatted methods and updated default values for improved consistency.
`.../pharein/restarts.py`	Modified logic for handling restart options for improved data consistency.
`.../pharein/simulation.py`	Changes in various methods for improved simulation setup and data handling.
`.../pharein/uniform_model.py`	Reformatting of methods and default arguments for better code structure.
`.../pharesee/geometry.py`	Adjustments to function declarations, variable assignments, and conditional checks for improved logic and control flow.
`.../pharesee/hierarchy.py`	Modified internal logic of functions for improved data processing and control flow.
`.../pharesee/particles.py`	Reformatting of class initialization and method adjustments for improved functionality.
`.../pharesee/plotting.py`	Changes in variable assignments, histogram generation, and conditional blocks for improved plotting logic.
`.../pharesee/run.py`	Modifications to various methods for improved data handling and computation.
`.../simulator/_simulator.py`	Removal of comments and minor variable assignment adjustments.
`.../simulator/simulator.py`	Minor formatting changes, variable renaming, and comment adjustments.
`.../pharein/simulation_test.py`	Modifications to function declarations and their usage within the test class.
`.../test_core/test_box.py`	Changes in test data and formatting of test methods for improved readability.
`.../test_pharesee/__init__.py`	Removal of redundant whitespace and adjustments to variable assignments.
`.../test_pharesee/test_geometry.py`	Removal of redundant lines and indentation adjustments for improved code structure.
`.../test_pharesee/test_geometry_2d.py`	Removal of blank lines for improved code structure.
`src/phare/phare_init.py`	Modified import statements, function parameters, and dictionary formatting for improved code structure.
`src/phare/phare_init_small.py`	Changes in import statements, function parameters, and dictionary formatting for improved code structure.
`tests/amr/data/field/coarsening/test_coarsen_field.py`	Removal of empty lines within functions for improved code structure.
`tests/core/data/gridlayout/allocSizes.py`	Consistent spacing and parentheses in conditional statements, and consistent spacing and quotes in function parameters.
`tests/core/data/gridlayout/cellCenteredCoordinates.py`	Refactoring for improved readability and maintainability, including reformatting conditional statements, adjusting indentation, and simplifying conditional blocks.
`tests/core/data/gridlayout/deriv.py`	Modifications to method implementations, reformatting of lists and variable assignments, and adjustments to file opening methods for improved code structure.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ?

Tips

Chat with CodeRabbit Bot (`@coderabbitai`)

You can reply to a review comment from CodeRabbit.
You can also chat with CodeRabbit bot directly around the specific lines of code or files in the PR by tagging @coderabbitai in a new comment.
You can tag @coderabbitai in a PR comment and ask questions about the PR and the codebase.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
The JSON schema for the configuration file is available here.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

src/phare/phare_init.py

src/phare/phare_init_small.py

pyphare/pyphare/pharesee/geometry.py

            # if periodic, always true for now
-            if True:  #lgtm [py/constant-conditional-expression]
+            if True:  # lgtm [py/constant-conditional-expression]


pyphare/pyphare/pharein/init.py

    _init_.PHARE_EXE = True
    print(jobname)
-    jobmodule = importlib.import_module(jobname) #lgtm [py/unused-local-variable]
+    jobmodule = importlib.import_module(jobname)  # lgtm [py/unused-local-variable]


pyphare/pyphare/pharesee/plotting.py

        ax.set_aspect("equal")
        divider = make_axes_locatable(ax)
        cax = divider.append_axes("right", size="5%", pad=0.08)
-        cb = plt.colorbar( im, ax=ax, cax=cax )
+        cb = plt.colorbar(im, ax=ax, cax=cax)


pyphare/pyphare/pharesee/particles.py

-
-
+        z1 = particles.iCells[:, 2] + particles.deltas[:, 2]
+        return np.argsort(np.sqrt((x1**2 + y1**2 + z1**2)) / (x1 / y1 / z1))


pyphare/pyphare/pharesee/plotting.py

-              weights=particles.weights[:,0])
+    bins = kwargs.get("bins", (50, 50))
+    h, xh, yh = np.histogram2d(
+        x, y, bins=kwargs.get("bins", bins), weights=particles.weights[:, 0]


pyphare/pyphare/pharesee/plotting.py

-              weights=particles.weights[:,0])
+    bins = kwargs.get("bins", (50, 50))
+    h, xh, yh = np.histogram2d(
+        x, y, bins=kwargs.get("bins", bins), weights=particles.weights[:, 0]


pyphare/pyphare/pharesee/plotting.py

@@ -90,15 +94,16 @@
    elif color_scale == "linear":
        norm = Normalize(cmin, cmax)

-    im = ax.pcolormesh(xh, yh, image, cmap = cmap, norm = norm)
+    im = ax.pcolormesh(xh, yh, image, cmap=cmap, norm=norm)


PhilipDeegan · 2023-12-12T10:37:57Z

not sure we really care about most of these warnings

coderabbitai

Review Status

Actionable comments generated: 25

Configuration used: .coderabbit.yaml

Commits

Files that changed from the base of the PR and between 2e9c825 and cf1fcff.

Files selected for processing (57)

pyphare/pyphare/core/box.py (7 hunks)
pyphare/pyphare/core/gridlayout.py (3 hunks)
pyphare/pyphare/core/phare_utilities.py (7 hunks)
pyphare/pyphare/cpp/init.py (2 hunks)
pyphare/pyphare/data/wrangler.py (2 hunks)
pyphare/pyphare/pharein/init.py (9 hunks)
pyphare/pyphare/pharein/diagnostics.py (4 hunks)
pyphare/pyphare/pharein/electron_model.py (1 hunks)
pyphare/pyphare/pharein/examples/job.py (6 hunks)
pyphare/pyphare/pharein/global_vars.py (1 hunks)
pyphare/pyphare/pharein/init.py (1 hunks)
pyphare/pyphare/pharein/maxwellian_fluid_model.py (4 hunks)
pyphare/pyphare/pharein/restarts.py (4 hunks)
pyphare/pyphare/pharein/simulation.py (16 hunks)
pyphare/pyphare/pharein/uniform_model.py (3 hunks)
pyphare/pyphare/pharesee/geometry.py (15 hunks)
pyphare/pyphare/pharesee/hierarchy.py (13 hunks)
pyphare/pyphare/pharesee/particles.py (6 hunks)
pyphare/pyphare/pharesee/plotting.py (12 hunks)
pyphare/pyphare/pharesee/run.py (7 hunks)
pyphare/pyphare/simulator/_simulator.py (1 hunks)
pyphare/pyphare/simulator/simulator.py (5 hunks)
pyphare/pyphare_tests/pharein/simulation_test.py (1 hunks)
pyphare/pyphare_tests/test_core/test_box.py (3 hunks)
pyphare/pyphare_tests/test_pharesee/init.py (5 hunks)
pyphare/pyphare_tests/test_pharesee/test_geometry.py (5 hunks)
pyphare/pyphare_tests/test_pharesee/test_geometry_2d.py (2 hunks)
src/phare/phare_init.py (3 hunks)
src/phare/phare_init_small.py (3 hunks)
tests/amr/data/field/coarsening/test_coarsen_field.py (2 hunks)
tests/core/data/gridlayout/allocSizes.py (2 hunks)
tests/core/data/gridlayout/cellCenteredCoordinates.py (4 hunks)
tests/core/data/gridlayout/deriv.py (4 hunks)
tests/core/data/gridlayout/fieldNodeCoordinates.py (2 hunks)
tests/core/data/gridlayout/gridIndexing.py (1 hunks)
tests/core/data/gridlayout/grid_yee.py (1 hunks)
tests/core/data/gridlayout/gridlayout_generate_test_files.py (1 hunks)
tests/core/data/gridlayout/gridparams.py (1 hunks)
tests/core/data/gridlayout/test_deriv.py (3 hunks)
tests/core/data/gridlayout/test_laplacian.py (4 hunks)
tests/core/data/gridlayout/test_linear_combinaisons_yee.py (2 hunks)
tests/core/data/gridlayout/utilities.py (1 hunks)
tests/core/numerics/ampere/test_ampere.py (5 hunks)
tests/core/numerics/faraday/test_faraday.py (5 hunks)
tests/core/numerics/interpolator/interpolator_test.py (4 hunks)
tests/core/numerics/ohm/test_ohm.py (5 hunks)
tests/core/numerics/pusher/test_pusher.py (1 hunks)
tests/diagnostic/init.py (2 hunks)
tests/functional/alfven_wave/alfven_wave1d.py (4 hunks)
tests/functional/conservation/conserv.py (8 hunks)
tests/functional/dispersion/dispersion.py (1 hunks)
tests/functional/harris/harris_2d.py (1 hunks)
tests/functional/ionIonBeam/ion_ion_beam1d.py (4 hunks)
tests/functional/shock/shock.py (1 hunks)
tests/functional/td/td1d.py (1 hunks)
tests/functional/tdtagged/td1dtagged.py (4 hunks)
tests/functional/translation/translat1d.py (1 hunks)

Files not processed due to max files limit (4)

tests/simulator/config.py
tests/simulator/refinement/test_2d_10_core.py
tests/simulator/refinement/test_2d_2_core.py
tests/simulator/test_restarts.py

Files not summarized due to errors (1)

tests/core/numerics/ohm/test_ohm.py: Error: Message exceeds token limit

Files skipped from review due to trivial changes (14)

pyphare/pyphare/core/gridlayout.py
pyphare/pyphare/pharein/electron_model.py
pyphare/pyphare/pharein/global_vars.py
pyphare/pyphare/simulator/_simulator.py
pyphare/pyphare/simulator/simulator.py
pyphare/pyphare_tests/test_core/test_box.py
pyphare/pyphare_tests/test_pharesee/init.py
pyphare/pyphare_tests/test_pharesee/test_geometry.py
pyphare/pyphare_tests/test_pharesee/test_geometry_2d.py
tests/amr/data/field/coarsening/test_coarsen_field.py
tests/core/data/gridlayout/gridlayout_generate_test_files.py
tests/core/data/gridlayout/test_deriv.py
tests/core/numerics/faraday/test_faraday.py
tests/core/numerics/interpolator/interpolator_test.py

Additional comments: 280

pyphare/pyphare/core/box.py (7)

11-17: The changes in the __init__ method of the Box class, specifically the casting of self.lower and self.upper to integer types, are more than just formatting changes. This could potentially alter the behavior of the class if it was previously handling non-integer types. Please verify that this change is intended and does not introduce any regressions or unexpected behavior in the codebase.

58-62: The updated comparison logic in the __eq__ method of the Box class is a substantial change and not just a formatting update. Ensure that this new logic correctly implements the equality comparison and that all dependent code has been updated to handle this new behavior if necessary.

28-48: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [46-55]

The logic in the __contains__ method has been adjusted. This is a significant change that could affect how containment is determined within the Box class. Please verify that this change is correct and does not introduce any regressions or unexpected behavior.

116-116: The addition of the grow method is a new feature and not just a formatting change. Please ensure that this method is correctly implemented and that its integration with the existing codebase has been thoroughly tested.

119-121: The addition of the shrink method is another new feature. Verify that this method is correctly implemented and that its integration with the existing codebase has been thoroughly tested.

166-173: The logic in the remove function has been altered, specifically in the handling of the "back" and "front" boxes. This is a significant change that could affect the behavior of the function. Please verify that this change is correct and does not introduce any regressions or unexpected behavior.

28-48: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [5-173]

The PR title "formatting python via black" suggests that the changes should be purely formatting, but the changes reviewed in box.py include significant logic alterations and the addition of new methods. Please ensure that the PR title and description accurately reflect the nature of the changes made to avoid confusion.

pyphare/pyphare/core/phare_utilities.py (6)

57-60: The function np_array_ify has been refactored. Ensure that the new logic correctly handles the case when arg is a scalar by creating an array with size number of arg elements.

128-138: The run_cli_cmd function has been refactored for readability. Verify that the subprocess call and exception handling remain functionally equivalent to the previous implementation.

141-143: The git_hashes function has been refactored for readability. Verify that the subprocess call and decoding of bytes remain functionally equivalent to the previous implementation.

146-150: The top_git_hash function has been refactored for readability. Verify that the subprocess call and decoding of bytes remain functionally equivalent to the previous implementation.

154-158: The print_trace function has been refactored for readability. Verify that the traceback printing remains functionally equivalent to the previous implementation.

170-172: The deep_copy function has been refactored for readability. Verify that the deep copy logic remains functionally equivalent to the previous implementation.
pyphare/pyphare/cpp/__init__.py (2)
4-4: The addition of a blank line adheres to PEP 8's recommendation for two blank lines before a top-level function definition, which is good for readability.

32-33: The create_splitter function has been added. Ensure that this new function is correctly implemented and used wherever necessary in the codebase.
#!/bin/bash
# Search for the usage of the new `create_splitter` function to ensure it's being used correctly.
ast-grep --lang python --pattern $'create_splitter($_, $_, $_)'
pyphare/pyphare/data/wrangler.py (5)
4-15: The instantiation of self.cpp using getattr and the dynamic attribute name is a significant change. Verify that the cpp_lib function returns an object that has the expected DataWrangler class with the correct dimension, interpolation order, and refined particle number. Also, ensure that the simulator.cpp_sim and simulator.cpp_hier are valid arguments for the constructor of this dynamically fetched class.

17-18: The kill method has been removed from the DataWrangler class. Verify that this method is not called from anywhere else in the codebase to prevent potential runtime errors.
#!/bin/bash
# Search for references to the `kill` method in the `DataWrangler` class.
rg 'DataWrangler.*kill'
48-48: A new method lvl0PopFluxes has been added to the DataWrangler class. Ensure that this method is correctly implemented and check if there are any places in the codebase where this new method should be utilized.
#!/bin/bash
# Search for potential usage of the new `lvl0PopFluxes` method.
rg 'lvl0PopFluxes'
50-52: The extract_is_primal_key_from method has been refactored to change how the key is extracted from the string. Verify that this new implementation correctly extracts the intended key and does not introduce any logical errors.
#!/bin/bash
# Search for usage of the `extract_is_primal_key_from` method to ensure it's being used correctly.
rg 'extract_is_primal_key_from'
54-64: The lvl0EM method has been refactored, with changes to the sync_merge call and its arguments. Verify that the use of gridlayout.yee_element_is_primal and the new method of determining if an element is primal are correct and do not introduce any logical errors.
#!/bin/bash
# Search for usage of the `lvl0EM` method to ensure the changes are correctly applied.
rg 'lvl0EM'
pyphare/pyphare/pharein/__init__.py (8)
13-26: The changes in this hunk seem to be formatting the output of sys.path into a more readable form by removing unnecessary characters. This is in line with the Black formatter's style guide.

35-45: The import statements have been reorganized into multiple lines for better readability. This is a common practice for long import lists and is consistent with PEP 8 style guidelines.

54-71: The addition of _patch_data_ids and _serialized_simulation_string functions indicates new functionality rather than just formatting. This needs to be verified to ensure that it aligns with the PR's objective of only formatting changes and does not introduce new behavior.
#!/bin/bash
# Verify that the new functions are used correctly and do not introduce new behavior.
rg '_patch_data_ids|_serialized_simulation_string' -- 'pyphare/pyphare/pharein/*.py'
76-82: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [76-89]

The py_fn_wrapper class and its __call__ method have been reformatted. The added blank lines (79 and 97) are likely for readability and separation of code blocks, which is consistent with Python's style guidelines.

106-112: The clearDict function has been modified with a comment explaining the purpose of the function. This is a good practice for code maintainability and readability.

115-175: The populateDict function has been reformatted with additional spacing and comments. It's important to ensure that the logic has not been altered, especially since the function seems to be setting up a configuration dictionary for a simulation.

223-298: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [177-314]

The changes in this hunk involve restructuring the way refinement boxes are handled and adding error handling for restart files. These changes appear to go beyond simple formatting and may introduce new behavior or logic changes.
#!/bin/bash
# Verify that the changes to refinement box handling and restart file error handling do not introduce new behavior.
rg 'refinement_boxes|restart_file_load_path|deserialized_simulation' -- 'pyphare/pyphare/pharein/*.py'
223-298: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [223-357]

The changes in this hunk involve modifications to the way particle initializers and diagnostics are set up. The addition of new functions and the restructuring of existing code should be verified to ensure they are consistent with the PR's formatting objective and do not introduce new behavior.
#!/bin/bash
# Verify that the changes to particle initializers and diagnostics setup do not introduce new behavior.
rg 'partinit|diag_path|restarts_path|serialized_simulation' -- 'pyphare/pyphare/pharein/*.py'
pyphare/pyphare/pharein/diagnostics.py (3)

7-29: The changes in the diagnostics_checker function, including the use of double quotes and the restructuring of error messages, align with the PR's objective of code formatting and readability improvements.

57-335: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [52-74]

The updates to the validate_timestamps function, including the addition of detailed error messages and formatting adjustments, are consistent with the PR's objective and enhance error message clarity.

91-335: The modifications in the Diagnostics class and its subclasses, such as the use of double quotes, adjustments in error messages, and changes in conditional checks, are consistent with the PR's objective of code formatting and readability improvements.

pyphare/pyphare/pharein/examples/job.py (1)

111-112: Verify if the print statement is intended to be part of the final code, as it seems to be for debugging purposes.

pyphare/pyphare/pharein/init.py (2)

6-9: The setting of _init_.PHARE_EXE to True appears to be a logical change rather than a formatting one. Please confirm that this change is intentional and verify its impact on the behavior of the get_user_inputs function and any downstream effects.

8-8: The variable jobmodule is flagged with a comment to suppress a warning about it being unused. Since this PR is focused on formatting, no action is required, but it's worth noting that the variable remains unused. If this is not intentional, consider addressing the unused variable in a future change.
pyphare/pyphare/pharein/maxwellian_fluid_model.py (5)
9-24: The changes to the lambda functions in the defaulter method to use the input value are consistent with the PR's objective to improve formatting and readability. The use of x * 0 ensures that the lambda function returns a value of the same shape as x, which is a good practice for ensuring consistency in array operations.

26-40: The changes to the default values for bx, by, and bz in the __init__ method from integers to floats are significant. This could potentially change the behavior of the code if the type was important for the calculations. It's important to ensure that downstream code that uses these values is expecting floats and not integers.
#!/bin/bash
# Verify that the downstream code expects floats for bx, by, and bz.
ast-grep --lang python --pattern $'MaxwellianFluidModel($_, $_, $_, $$$)'
60-74: The add_population method has been reformatted, and the default values for charge, mass, and other parameters have been changed from integers to floats. This is consistent with the PR's objective and should not affect the behavior as these are more likely to be floats in a physical context.

136-137: The to_dict method has been modified to include the number of ion populations in the model dictionary. This is a functional change and should be checked to ensure that it aligns with the expected behavior of the method and that the consumers of this method are expecting this new key-value pair.
#!/bin/bash
# Verify that the consumers of the to_dict method expect the new key-value pair.
rg 'to_dict' -- '-A 3' | grep 'nbr_ion_populations'
142-147: The validate method has been reformatted for better readability. The changes are purely stylistic and do not affect the logic of the method.
pyphare/pyphare/pharein/restarts.py (4)
1-3: The import statement for os is present, but according to the summary, it is no longer used. If this is the case, it should be removed to keep the code clean and efficient, aligning with the user's preference for minimal code.
#!/bin/bash
# Search for usage of `os` module in `restarts.py`
rg 'os\.' -- 'pyphare/pyphare/pharein/restarts.py'
9-25: The logic for converting elapsed_timestamps to seconds and ensuring they are in ascending order looks correct. This change improves data consistency and error handling.

33-59: The validation logic for timestamps is comprehensive, checking for correct ordering, bounds within the simulation time, and alignment with the simulation time step. This ensures robust error handling and data integrity.

67-87: The conserve_existing function's logic appears to be correct, as it checks for the existence of restart files and removes timestamps accordingly. This is an efficient way to handle file existence checks and aligns with the user's preference for efficient code.
pyphare/pyphare/pharein/simulation.py (14)

26-50: The changes in the check_domain function introduce new logic for checking the presence of keyword arguments. This goes beyond formatting and could potentially alter the behavior of the function. Please confirm that these changes were intended and that they have been tested to ensure they do not introduce any regressions.

681-714: The added documentation and examples in the Simulation class are helpful for understanding how to use the class and its methods. This is a good improvement in terms of code readability and maintainability.

578-610: The checker decorator function now includes a list of accepted keywords, which is a significant change from the previous version. This could potentially alter the behavior of the functions it decorates. Please confirm that these changes were intended and that they have been tested to ensure they do not introduce any regressions.

910-920: The serialize and deserialize functions have been modified to use codecs.encode and codecs.decode, respectively. This is a logical change and not just a formatting one. Please confirm that these changes were intended and that they have been tested to ensure they do not introduce any regressions.

847-849: The restart_file_path method in the Simulation class has been modified to use the restart_options dictionary. This is a logical change and not just a formatting one. Please confirm that these changes were intended and that they have been tested to ensure they do not introduce any regressions.

422-423: The check_directory function has been modified to strip trailing slashes from the directory path. This is a logical change and not just a formatting one. Please confirm that these changes were intended and that they have been tested to ensure they do not introduce any regressions.

298-303: The check_refinement_boxes function has been modified to return None, 1 if refinement_boxes is None or empty. This is a logical change and not just a formatting one. Please confirm that these changes were intended and that they have been tested to ensure they do not introduce any regressions.

233-247: The check_refined_particle_nbr function has been modified to use a default value from valid_refined_particle_nbr. This is a logical change and not just a formatting one. Please confirm that these changes were intended and that they have been tested to ensure they do not introduce any regressions.

358-365: The check_patch_size function has been modified to include a new get_max_ghosts function. This is a logical change and not just a formatting one. Please confirm that these changes were intended and that they have been tested to ensure they do not introduce any regressions.

871-880: The add_diagnostics method in the Simulation class has been modified to include a check for duplicate diagnostic names. This is a logical change and not just a formatting one. Please confirm that these changes were intended and that they have been tested to ensure they do not introduce any regressions.

557-562: The check_hyper_resistivity function has been modified to include a default value. This is a logical change and not just a formatting one. Please confirm that these changes were intended and that they have been tested to ensure they do not introduce any regressions.

565-572: The check_clustering function has been modified to include a check for valid clustering types. This is a logical change and not just a formatting one. Please confirm that these changes were intended and that they have been tested to ensure they do not introduce any regressions.

817-827: The Simulation class has been modified to include new attributes and calculations for nSubcycles, stepDiff, level_time_steps, and level_step_nbr. This is a logical change and not just a formatting one. Please confirm that these changes were intended and that they have been tested to ensure they do not introduce any regressions.

844-845: The within_simulation_extent method in the Simulation class has been modified to raise a NotImplementedError for 2D and 3D cases. This is a logical change and not just a formatting one. Please confirm that these changes were intended and that they have been tested to ensure they do not introduce any regressions.

pyphare/pyphare/pharein/uniform_model.py (1)

4-8: The changes to the __init__ method's default arguments from tuples containing floats to tuples with explicit float literals are consistent with the Black formatter's preference for explicit decimal points. This change should not affect the behavior of the code.
pyphare/pyphare/pharesee/geometry.py (8)
1-3: Renaming boxm to boxm_mod in the import statement could potentially break the code if boxm is used elsewhere in the file or in other files that import this module. Verify that all instances of boxm have been updated to boxm_mod to maintain consistency.
#!/bin/bash
# Search for 'boxm' usage in the current file to ensure it's been updated to 'boxm_mod'.
rg 'boxm' 'pyphare/pyphare/pharesee/geometry.py'
# Search for 'boxm' usage in other files that might import this module.
rg 'from .geometry import boxm'
10-27: The changes in the toFieldBox function seem to be purely formatting, such as removing redundant parentheses and adding comments. This aligns with the PR's objective of applying Black formatting and should not affect the function's logic.

37-68: The calculation of max_ghosts has been modified to use a more Pythonic list comprehension and max function. This is a refactoring change that should not alter the logic but should be tested to ensure it produces the same results as before.

80-146: The periodicity_shifts function has been refactored to include more cases and handle 2D shifts. This goes beyond simple formatting and could introduce logic changes. Ensure that the new cases are covered by tests and that the function behaves as expected in all scenarios.
#!/bin/bash
# Search for tests related to 'periodicity_shifts' to ensure coverage of new cases.
rg 'test.*periodicity_shifts' -- 'tests/'
204-235: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [186-217]

The changes in the compute_overlaps function are formatting-related, such as adjusting indentation and adding comments. The logic appears to remain the same, which is consistent with the PR's formatting objective.

318-363: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [318-370]

The changes in the get_periodic_list function include formatting adjustments and the addition of comments. However, the logic for handling domain borders and shifting patches seems to have been modified. Verify that these changes are correct and do not introduce any unintended behavior.
#!/bin/bash
# Search for tests related to 'get_periodic_list' to ensure the logic changes are tested.
rg 'test.*get_periodic_list' -- 'tests/'
408-416: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [372-415]

The ghost_area_boxes function has been updated with formatting changes and some variable renaming. The logic appears to be unchanged, which is consistent with the PR's objective.

497-498: The return statement has been modified to return a specific level's ghost boxes if levelNbrs is a scalar. This seems to be a logic change and should be verified to ensure it aligns with the intended functionality.
#!/bin/bash
# Search for usage of 'level_ghost_boxes' to ensure the logic change is accounted for.
rg 'level_ghost_boxes' -- 'tests/'
pyphare/pyphare/pharesee/hierarchy.py (9)

171-177: The inner function grid has been moved outside of meshgrid and is now a standalone function. This change is not just formatting; it alters the scope of grid. Verify that this change is intentional and does not affect the expected behavior of meshgrid.

404-413: The loop in flat_finest_field_1d has been refactored to remove all but one ghost node to limit overlapping. This is a logic change and not just formatting. Ensure that this change is correct and does not introduce any bugs or unintended behavior.

444-453: The loop in flat_finest_field_2d has been refactored to remove all but one ghost node to limit overlapping. This is a logic change and not just formatting. Ensure that this change is correct and does not introduce any bugs or unintended behavior.

525-530: The loop in finest_part_data has been refactored to keep all particles at the finest level and only those not overlapped by finer boxes at other levels. This is a logic change and not just formatting. Ensure that this change is correct and does not introduce any bugs or unintended behavior.

1078-1083: The loop in amr_grid has been refactored to create a non-uniform contiguous primal grid associated with the given hierarchy. This is a logic change and not just formatting. Ensure that this change is correct and does not introduce any bugs or unintended behavior.

1256-1259: The code for reshaping the velocity array v in add_to_patchdata assumes that the size of v is divisible by 3. This could potentially lead to errors if v.size is not a multiple of 3. Verify that this assumption is always valid.

1343-1347: The loop in hierarchy_fromh5 creates patch levels from an HDF5 file. This is a logic change and not just formatting. Ensure that this change is correct and does not introduce any bugs or unintended behavior.

1499-1504: The assertion in hierarchy_from_sim checks the dimensionality of the domain box against the simulator's domain box dimensionality. This is a logic change and not just formatting. Ensure that this assertion is correct and necessary.

1631-1634: The merge_particles function has been added to merge domain, patchGhost, and levelGhost particles. This is a significant logic change and not just formatting. Ensure that this change is correct and does not introduce any bugs or unintended behavior.

pyphare/pyphare/pharesee/particles.py (12)

11-32: The __init__ method has been updated with new initialization logic for several attributes. Ensure that these changes are consistent with the expected behavior of the class and that the logic correctly initializes the attributes based on the provided arguments.

43-43: The xyz method has been modified. Verify that the changes are purely formatting and do not alter the logic of the method.

62-67: The add method now concatenates attributes from another Particles instance. Ensure that the concatenation is done correctly and that the _reset method is called to reset any cached properties.

101-127: The select method has been modified with additional logic for selecting particles. Verify that the selection logic is correct and that the assertions are appropriate for the intended behavior.

133-138: The erase method has been modified to delete elements from arrays. Ensure that the deletion logic is correct and that it does not introduce any unintended side effects.

142-147: The pop method has been modified to copy and return a subset of particles. Verify that the copying is done correctly and that the erase method is called to remove the selected particles from the original instance.

152-170: The split method has been modified with new logic for reshaping arrays. Ensure that the reshaping is done correctly and that the logic is consistent with the simulation parameters.

174-182: The as_tuples method has been modified. Verify that the tuple creation is correct and that the size method is used appropriately to iterate over the particles.

193-204: The all_assert_sorted function has been modified with new tolerance values for assertions. Ensure that the tolerance values are appropriate and that the assertions are correct for comparing particle attributes.

225-230: The aggregate function has been modified. Verify that the aggregation of particles is done correctly and that the size assertion is appropriate.

241-259: The remove function has been modified to delete elements from arrays and reconstruct the v array. Ensure that the removal and reconstruction logic is correct and that the new Particles instance is created with the correct attributes.

263-274: The _arg_sort function has been modified with new sorting logic. Verify that the sorting logic is correct and that the dimensionality checks are appropriate for the intended behavior.

pyphare/pyphare/pharesee/plotting.py (8)

1-6: The reformatting of import statements to be on separate lines aligns with PEP 8 style guidelines and improves readability.

50-83: The changes in the dist_plot function appear to be formatting changes, but it's important to ensure that the logic for histogram generation and filtering remains intact after the reformatting.

148-153: The reformatting of the connect_bbox function should not affect its behavior, but it's important to verify that the properties passed to BboxConnectorPatch are still correctly applied.

204-215: The reformatting of the zoom_effect function should maintain the original functionality, but it's crucial to verify that the properties passed to the connect_bbox function are still correctly applied.

252-296: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [226-293]

The changes in the finest_field_plot function seem extensive and include modifications in variable assignments, conditional blocks, and plotting logic. It's essential to verify that these changes are purely formatting and do not introduce any new behavior or alter existing behavior.

65-65: Please verify if the potential issue of uninitialized local variable 'x' flagged by the security bot has been addressed in this PR.

97-97: Please verify if the potential issue of uninitialized local variable 'norm' flagged by the security bot has been addressed in this PR.

329-329: Please verify if the issue of the unused local variable 'cb' flagged by the security bot has been addressed in this PR.

pyphare/pyphare/pharesee/run.py (10)

14-26: The changes in _current1d function appear to be more than just formatting; they include logic changes for calculating jy and jz. Ensure that these changes are intended and correctly implemented, as they could affect the function's behavior.

39-61: Similar to _current1d, the _current2d function has logic changes for calculating jx, jy, and jz. Verify that these changes are correct and align with the intended behavior of the function.

66-102: The _compute_current function now includes separate handling for 1D and 2D cases, which is a significant change from the previous version. Confirm that this new implementation is correct and that it has been tested to ensure it works as expected.

105-122: The _compute_divB function has been updated to handle 2D cases, and a check has been added for 1D cases to raise a ValueError. This is a logic change and should be verified for correctness.

128-147: The _get_rank function has been modified to include changes in the way the MPI rank is computed and returned. This is a logic change and should be verified for correctness.

158-193: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [150-193]

The make_interpolator function has been updated with changes to the interpolator creation and the calculation of finest_coords. These changes are significant and should be verified for correctness.

200-215: The Run class constructor and _get_hierarchy method have been updated. The constructor now initializes self.hier to None, and _get_hierarchy has been refactored. These changes should be verified to ensure they do not introduce any unintended side effects.

227-238: The changes in the _get method of the Run class involve logic changes for creating interpolators for merged quantities. Verify that these changes are correct and that they have been tested.

244-321: The Get* methods in the Run class have been updated with changes to the way hierarchies are retrieved and quantities are computed. These changes are significant and should be verified for correctness.

331-357: The GetDl and GetAllAvailableQties methods in the Run class have been updated. The GetDl method now includes changes to the way the grid sizes are calculated, and GetAllAvailableQties has an added assertion. These changes should be verified for correctness.

pyphare/pyphare_tests/pharein/simulation_test.py (4)

17-21: The reformatting of the bcs list enhances readability and is consistent with the Black formatter's style guide.

28-45: The changes in the test_dl method, including the additional logic for handling iterable domain_size and cells, are consistent with the Black formatter's style guide and enhance the code's readability and functionality.

7-7: The addition of from pyphare.core import phare_utilities is not mentioned in the AI-generated summary. Please verify if this import is necessary for the changes made in this file.

13-26: The updates to the setUp method, including the addition of domain_size_array, ndim, bcs, and final_time, are consistent with the Black formatter's style guide and enhance the code's readability and functionality.

src/phare/phare_init.py (14)

5-6: The import statement has been modified to use an alias for pyphare.pharein. This is a cosmetic change and should not affect functionality as long as the alias ph is consistently used throughout the file.

9-20: The Simulation class instantiation has been reformatted with additional parameters and values have been changed to be more explicit (e.g., 30. to 30.0). This is a formatting change that improves readability and does not affect functionality.

24-25: The density function has been updated to return a floating-point number. This change is consistent with the PR's objective to standardize the formatting and should not affect functionality as long as the return type is expected to be a float.

28-32: The by function has been reformatted and now includes a floating-point multiplication. This change is in line with the PR's objective and should not affect functionality.

35-39: The bz function has been reformatted and now includes a floating-point multiplication. This change is in line with the PR's objective and should not affect functionality.

42-43: The bx function now returns a floating-point number. This change is consistent with the PR's objective to standardize the formatting and should not affect functionality.

46-47: The vx function now returns a floating-point number. This change is consistent with the PR's objective to standardize the formatting and should not affect functionality.

50-54: The vy function has been reformatted and now includes a floating-point multiplication. This change is in line with the PR's objective and should not affect functionality.

57-61: The vz function has been reformatted and now includes a floating-point multiplication. This change is in line with the PR's objective and should not affect functionality.

76-82: The vvv dictionary has been reformatted to have each key-value pair on a new line. This is a formatting change that improves readability and does not affect functionality.

85-89: The MaxwellianFluidModel instantiation has been reformatted to use the ph prefix for the class names. This change is consistent with the updated import statement and should not affect functionality.

92-92: The ElectronModel instantiation has been reformatted to use the ph prefix for the class names. This change is consistent with the updated import statement and should not affect functionality.

97-97: The timestamps array initialization has been modified to include spaces around the arithmetic operators. This is a formatting change that improves readability and does not affect functionality.

115-117: The pops list has been reformatted to have each element on a new line. This is a formatting change that improves readability and does not affect functionality.

src/phare/phare_init_small.py (6)

5-6: The import statement has been modified to use an alias for pyphare.pharein. This is a cosmetic change and should not affect functionality.

9-23: The Simulation class is now being accessed using the ph alias. This change is consistent with the import statement change and should not affect functionality.

27-27: The density function has been updated. Ensure that this change is consistent with the intended formatting changes and does not introduce any functional differences.

88-92: The MaxwellianFluidModel is now being accessed using the ph alias. This change is consistent with the import statement change and should not affect functionality.

95-95: The ElectronModel is now being accessed using the ph alias. This change is consistent with the import statement change and should not affect functionality.

100-100: The timestamps variable has been updated. Ensure that this change is consistent with the intended formatting changes and does not introduce any functional differences.

tests/core/data/gridlayout/allocSizes.py (8)

17-28: The changes in the AllocParams class constructor and the setAlloc method appear to be formatting adjustments for consistency. Ensure that these changes align with the Black code formatter's standards and that no syntax errors have been introduced.

30-60: The conditional blocks for dimension checks and allocation assignments in the setAlloc method have been reformatted. Verify that the logic remains intact and that the formatting is consistent with the Black code formatter's guidelines.

64-66: The main function's parameter path has been updated to use double quotes consistently. This change is purely stylistic and should not affect the functionality.

68-88: The interpOrders list and quantities list have been reformatted for consistency. Ensure that the changes are in line with the Black code formatter's standards and that no elements have been added or removed unintentionally.

90-98: The nbrCellXList, nbrCellYList, nbrCellZList, nbDimsList, dxList, dyList, and dzList have been reformatted for consistency. Confirm that the changes are in line with the Black code formatter's standards and that no values have been altered.

102-106: The baseName variable has been updated to use double quotes, and the commented-out file opening lines have been reformatted. These changes are stylistic and should not impact the functionality.

111-146: The loop that generates filenames and writes output has been reformatted. Ensure that the changes are in line with the Black code formatter's standards and that the loop's logic remains unchanged.

148-150: The closing of files in the loop has been reformatted. Confirm that the changes are in line with the Black code formatter's standards and that all files are still being closed properly.

tests/core/data/gridlayout/cellCenteredCoordinates.py (10)

19-25: The addition of blank lines (line 22) seems to be for readability, which aligns with the Black formatter's style guide.

33-38: The refactoring of the setCoord method appears to maintain the original logic while improving readability. However, ensure that the subtraction of gl.isDual(centering) is correct and does not alter the expected behavior.

40-66: The conditional blocks for setting coordinates based on the dimension have been reordered and simplified. This change should be verified to ensure that it does not affect any dependent code that relies on the previous ordering or structure.

69-75: The getCellCentered function has been simplified to return a tuple of "primal" strings based on the dimension. This change seems to be a straightforward refactoring for clarity.

87-87: The calculation of x in the centeredCoords function has been reformatted for readability. Ensure that the logic remains consistent with the expected behavior.

95-114: The main function has been updated with variable assignments and list initializations for improved readability. This change should not affect the logic, but it's important to verify that the new default values and lists match the intended use cases.

127-132: The addition of baseNameValues and outValues seems to be for better organization and readability. Ensure that the new variable names are consistently used throughout the code.

135-144: The creation of file names and opening of files for writing has been refactored. It's important to ensure that the files are being opened and closed properly, and that the file names match the expected patterns.

146-254: The loop structure has been refactored for clarity. Ensure that the nested loops and the writing of output strings to files are functioning as intended and that the refactoring has not introduced any off-by-one errors or other logical issues.

256-259: The closing of files in a loop is a good practice to ensure that all file descriptors are properly closed. Verify that all files opened in the script are accounted for and closed.
tests/core/data/gridlayout/deriv.py (9)
26-28: The refactoring of the setCoord method call to use the base class method is a good practice for reducing code duplication. However, ensure that the change in parameters passed to the method does not affect the expected behavior.

104-106: Using os.path.join for file paths is a good practice for ensuring cross-platform compatibility.

118-139: The reformatting of the loop to unpack values from zip is more readable and aligns with Python's idiomatic style. Ensure that the order of the lists being zipped matches the expected order for the loop variables.

146-146: The instantiation of the params object with improved formatting is consistent with Python's style guide.

216-217: The reformatting of the outDerivedValuesString should be checked to ensure it matches the expected output format for the derived values file.
#!/bin/bash
# Verify that the output format for derived values is consistent across the codebase.
rg 'outDerivedValuesString' -- '-format'
212-213: The use of scipy.misc.derivative to calculate the derivative is appropriate. Verify that the dx and order parameters are correctly chosen for the context of the calculation.
#!/bin/bash
# Verify that the dx and order parameters are used consistently for scipy.misc.derivative across the codebase.
rg 'scipy.misc.derivative' -- '-parameters'
182-190: The reformatting of the loop to calculate function values is more readable. Ensure that the loop bounds and the calculation of coord are correct and consistent with the intended behavior.

224-228: Properly closing files at the end of the main function is a good practice to prevent resource leaks.

72-193: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [78-232]

Given the extensive changes in the main function, it is recommended to run existing tests to ensure that the refactoring has not introduced any regressions.
#!/bin/bash
# Run existing tests for the deriv.py script to ensure no regressions.
pytest tests/core/data/gridlayout/deriv.py
tests/core/data/gridlayout/fieldNodeCoordinates.py (8)
5-6: The commented-out import of math is no longer needed and has been correctly removed as part of the cleanup.

28-34: The changes in the fieldCoords function are purely formatting-related and align with Black's code style, such as using 0.0 instead of 0. and double quotes for strings.

39-41: Explicitly calling the parent class's __init__ method in FieldNodeCoordParams is a good practice and ensures clarity in the code.

44-54: The changes in the getQtyCentering function are purely formatting-related and align with Black's code style, such as using double quotes for strings.

60-78: The changes in the main function, including the modification of default argument values and list initializations, are consistent with Black's formatting and do not introduce functional changes.

86-92: The changes to file I/O operations, such as adjusting file naming and writing operations, appear to be formatting changes. However, it's important to ensure that these changes do not affect the expected file paths or names used elsewhere in the codebase.
#!/bin/bash
# Verify that the new file naming convention does not conflict with other scripts or hardcoded paths.
rg 'fieldCoords_summary' --files-with-matches
rg 'fieldCoords_values' --files-with-matches
94-104: The changes in loop structures, such as adjusting loop variables and conditions, should be carefully reviewed to ensure they do not introduce off-by-one errors or other logical issues.
#!/bin/bash
# Verify that the loop adjustments do not introduce logical issues.
ast-grep --lang python --pattern $'for $_ in np.arange($_, $_ + 1) { $$$ }'
139-242: The changes in string formatting and function calls, such as adjusting the string formatting syntax, are consistent with Black's formatting and do not introduce functional changes.
tests/core/data/gridlayout/gridIndexing.py (4)
19-27: The changes to the IndexingParams class constructor appear to be purely formatting, adding spaces around commas as per PEP 8 guidelines.

28-42: The setIndexes method has been updated with double quotes for string literals, which is consistent with the Black formatter's style guide. Ensure that this change does not affect any string comparisons that might rely on specific quote types.
#!/bin/bash
# Search for string comparisons involving the affected literals in the `setIndexes` method.
ast-grep --lang python --pattern $'def setIndexes($_, $_) {
  $$$
  "X"
  $$$
  "Y"
  $$$
  "Z"
  $$$
}'
89-127: The main function's changes, including the use of double quotes and formatting of lists and variables, align with the Black formatter's style guide and PEP 8. No logical changes are observed.

132-172: The loop that generates file names and writes to the output files has been reformatted. It's important to ensure that the file paths and names are generated correctly and that the file operations are performed as expected.
#!/bin/bash
# Verify that the file paths and names are generated correctly.
rg --multiline 'outFilenameBase + "_" + str(dim) + "d_O" + str(interpOrder) + ".txt"'
# Verify that the file operations are performed correctly.
rg --multiline 'for f in files { f.close() }'
tests/core/data/gridlayout/grid_yee.py (1)

5-17: The changes in the InitValueValidation class, such as altering dictionary keys from single quotes to double quotes and the indentation of the test_1d method, are consistent with the Black code formatter's style guide. These changes enhance readability and maintain a consistent coding style across the codebase.
tests/core/data/gridlayout/gridparams.py (3)
2-4: The changes to the __init__ method of the GridParams class are consistent with the Black formatter's style guide.

8-14: The setNbrCell method has been updated to handle different grid dimensions. Ensure that the rest of the codebase expects self.nbrCell to be a tuple in all cases, as the single-dimensional case currently assigns a non-tuple value.
#!/bin/bash
# Search for usages of `self.nbrCell` to ensure it's always used as a tuple.
ast-grep --lang python --pattern $'self.nbrCell'
16-22: The setDl method has been updated similarly to setNbrCell. Verify that self.dl is expected to be a tuple in all cases for consistency across the codebase.
#!/bin/bash
# Search for usages of `self.dl` to ensure it's always used as a tuple.
ast-grep --lang python --pattern $'self.dl'
tests/core/data/gridlayout/test_laplacian.py (3)

11-24: The addition of JxCentering, JyCentering, and JzCentering in the TestVariables class is consistent with the formatting objective of the PR and appears to be used correctly in the subsequent test functions.

30-290: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [34-464]

The changes in the test functions test_laplacian_yee1D, test_laplacian_yee2D, and test_laplacian_yee3D are extensive, with modifications to variable assignments and array manipulations. These changes are in line with the PR's objective to improve formatting and do not seem to alter the underlying logic of the tests.

457-467: The addition of the main function and the if __name__ == "__main__": block is a standard practice for Python scripts and enhances the modularity and usability of the test script.

tests/core/data/gridlayout/test_linear_combinaisons_yee.py (6)

29-48: The commented code in the dualToPrimal and primalToDual functions has been reformatted for clarity. This does not affect the functionality as the code is commented out.

65-66: The addition of the tupleToString function enhances code readability by centralizing the conversion of tuples to strings.

335-359: The main function has been updated with improved formatting, including the use of double quotes and the reformatting of the cases dictionary to align keys and values.

361-369: The main function has been refactored to include a loop that iterates over the cases dictionary, writing output files for each case. This change improves the structure and readability of the code.

372-373: The addition of the if __name__ == "__main__": block is a standard practice in Python to ensure that the main function is called when the script is executed directly.

69-332: The functions ExToMoment, EyToMoment, EzToMoment, etc., have been refactored to use the tupleToString function for converting tuples to strings, which is consistent with the PR's objective of improving code readability and maintainability.

tests/core/numerics/ampere/test_ampere.py (5)

14-30: The reformatting of the TestVariables class constructor and the addition of comments improve readability and maintain the class's original functionality.

40-93: The reformatting and restructuring of the test_ampere_yee1D function maintain the original logic and improve readability. Ensure that the changes to array sizes and mathematical operations are consistent with the expected behavior of the test.

103-222: The changes in the test_ampere_yee2D function follow the same pattern as the test_ampere_yee1D function, with improved formatting and no apparent change in logic. The use of np.tensordot for generating Bx, By, and Bz and the subsequent calculations for Jx, Jy, and Jz appear to be correct.

292-421: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [234-400]

The test_ampere_yee3D function's changes are consistent with the other test functions, with the same improvements in formatting and readability. The calculations for Jx, Jy, and Jz using np.tensordot and array slicing are logically sound.

411-421: The main function and the conditional check for command-line arguments are correctly implemented. The function calls to test_ampere_yee1D, test_ampere_yee2D, and test_ampere_yee3D are preserved, ensuring that the script's functionality remains unchanged.

tests/core/numerics/ohm/test_ohm.py (5)

13-33: The introduction of the TestVariables class is a structural change, not just a formatting one. Confirm that this change aligns with the PR's objective of applying Black formatting and does not introduce any functional changes.

43-202: The changes in the test_ohm_yee1D function include the addition of numpy array initializations and calculations. Ensure that these changes are intended and do not introduce any functional changes that are outside the scope of formatting.

213-479: The changes in the test_ohm_yee2D function include the addition of numpy array initializations and calculations. Confirm that these changes are consistent with the PR's objective of formatting and do not introduce new functionality.

490-912: The changes in the test_ohm_yee3D function include the addition of numpy array initializations and calculations. Confirm that these changes are consistent with the PR's objective of formatting and do not introduce new functionality.

923-933: The changes to the main function and the if __name__ == "__main__": block appear to be more than just formatting. Confirm that these changes are intended and do not introduce any functional changes that are outside the scope of formatting.

tests/core/numerics/pusher/test_pusher.py (3)

11-50: The changes to the magnetic and electric field functions (Bx, By, Bz, Ex, Ey, Ez) and the deriv function are consistent with the PR's objective to apply Black formatting. The functions now return floating-point literals, which is a subtle change but ensures the return type is explicit and consistent.

56-67: The reformatting in the main function improves readability and consistency. The variable declarations and assignments are now more structured, and the calculation of nt and t is clearer. The use of np.arange and np.savetxt is consistent with numpy's best practices.

70-71: The change from single to double quotes in the if __name__ == "__main__": block is a minor stylistic change that aligns with the Black formatter's preference for double quotes.

tests/diagnostic/__init__.py (1)

3-3: The addition of blank lines improves readability and is consistent with the Black formatter's style guide.

Also applies to: 10-10

tests/functional/alfven_wave/alfven_wave1d.py (8)

3-15: The changes in this hunk reflect the application of Black formatting standards, such as the use of double quotes for strings and reorganization of imports. These changes are consistent with the PR's objective of code formatting and should not affect functionality.

26-39: The refactoring of the config function to use the ph namespace for simulation configuration is consistent with the PR's objective of improving code formatting and structure. Ensure that the ph namespace is correctly imported at the beginning of the file and that all instances where these classes and functions were used have been updated accordingly.

42-82: The updates to the function definitions appear to be formatting changes, such as the removal of redundant parentheses and the addition of spaces around operators. These changes should not affect the functionality of the code and are in line with the PR's objective.

85-105: The changes in the initialization of MaxwellianFluidModel and ElectronModel using the ph namespace are consistent with the PR's objective of improving code formatting and structure. Ensure that the ph namespace is correctly imported at the beginning of the file and that all instances where these classes and functions were used have been updated accordingly.

130-181: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [142-195]

The updates in the main function, including the use of the sim object returned from the config function and the initialization of the Simulator with this object, are consistent with the PR's objective of improving code formatting and structure. Ensure that the config function is correctly defined and returns the expected sim object.

3-9: The removal of specific imports from pyphare.pharein and the addition of ph as a namespace import is consistent with the PR's objective of improving code formatting and structure. Verify that the removed imports are no longer needed and that the ph namespace is used throughout the file.

130-181: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [142-198]

The minor formatting changes in the main function, such as the addition of spaces and reformatting of the plotting code, are consistent with the PR's objective of improving code formatting and structure. These changes should not affect the functionality of the code.

195-195: The addition of an assertion in the main function is a functional change. Ensure that this assertion is intended and correctly verifies the expected behavior of the code.
tests/functional/conservation/conserv.py (12)
3-6: Reorganized imports and direct usage of ph instead of pyphare.pharein are consistent with the PR's objective to improve code formatting and structure.

24-35: The creation of the Simulation object using ph.Simulation aligns with the updated import statements and does not alter the logic of the code.

77-81: The creation of MaxwellianFluidModel and ElectronModel objects using ph is consistent with the updated import statements and does not alter the logic of the code.

100-100: The uniform function now returns the sim object, which is a change in behavior. Verify if this change is intended and consistent with the rest of the codebase.
#!/bin/bash
# Search for usages of the `uniform` function to ensure they handle the new return value correctly.
ast-grep --lang python --pattern $'uniform($_, $_, $_, $_)'
209-218: The main function uses the updated Simulation and MaxwellianFluidModel objects, which is consistent with the updated import statements and does not alter the logic of the code.

115-125: The kinetic_energy function has been updated with new calculations. Verify if these changes are intended and if they affect the logic of the code.
#!/bin/bash
# Search for usages of the `kinetic_energy` function to ensure they are compatible with the new calculations.
ast-grep --lang python --pattern $'kinetic_energy($_, $_)'
136-142: The total_particles and mag_energy functions have been updated with new calculations. Verify if these changes are intended and if they affect the logic of the code.
#!/bin/bash
# Search for usages of the `total_particles` and `mag_energy` functions to ensure they are compatible with the new calculations.
ast-grep --lang python --pattern $'total_particles($_, $_, $_, $_)'
ast-grep --lang python --pattern $'mag_energy($_, $_)'
Also applies to: 156-178

189-197: The energies function now uses get_times_from_h5, which is a change in behavior. Verify if this change is intended and consistent with the rest of the codebase.
#!/bin/bash
# Search for usages of the `energies` function to ensure they handle the new behavior correctly.
ast-grep --lang python --pattern $'energies($_)'
200-201: The avg_interval function is new. Verify if this addition is intended and if it's consistent with the rest of the codebase.
#!/bin/bash
# Search for usages of the `avg_interval` function to ensure they are compatible with the new function.
ast-grep --lang python --pattern $'avg_interval($_, $_, $_)'
186-248: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [209-287]

The main function has been updated with new calculations and data processing logic. Verify if these changes are intended and if they affect the logic of the code.
#!/bin/bash
# Search for usages of the `main` function to ensure they are compatible with the new calculations and data processing logic.
ast-grep --lang python --pattern $'main()'
296-297: The addition of the if __name__ == "__main__": block is standard for Python scripts and does not alter the logic of the code.

3-6: Verify that the removal of imports such as pyphare.pharein.Simulation and others is intentional and does not break any dependencies within the codebase.
#!/bin/bash
# Search for removed imports to ensure they are no longer used in the codebase.
rg 'pyphare.pharein.Simulation'
rg 'pyphare.pharein.MaxwellianFluidModel'
rg 'pyphare.pharein.ElectronModel'
rg 'pyphare.pharein.ElectromagDiagnostics'
rg 'pyphare.pharein.FluidDiagnostics'
rg 'pyphare.pharein.ParticleDiagnostics'
tests/functional/dispersion/dispersion.py (8)
3-3: The import statement has been changed from pyphare.pharein to ph. This change should be verified to ensure that the ph module is correctly set up to be used as an alias for pyphare.pharein and that it does not affect any other parts of the codebase.
#!/bin/bash
# Verify that the alias 'ph' for 'pyphare.pharein' is correctly set up in the codebase.
rg 'import pyphare.pharein as ph' --files-with-matches
27-27: The value of final_time has been changed from 100. to 100.0. This change is cosmetic and does not affect the functionality, as both values represent the same floating-point number in Python.

33-36: The diag_options dictionary structure has been modified. This change appears to be a reformatting to adhere to the Black code style. It's important to ensure that the new structure is still compatible with the expected format for diag_options throughout the codebase.
#!/bin/bash
# Verify that the new structure of 'diag_options' is compatible with its usage in the codebase.
ast-grep --lang python --pattern $'$_ = {
  "diag_options": {
    "format": $_,
    "options": {
      "dir": $_,
      "mode": $_
    }
  }
}'
40-58: The return values of the functions density, by, bz, bx, vx, vy, and vz have been changed to return floating-point literals instead of integer literals. This change is a good practice for clarity, ensuring that the functions explicitly return floating-point numbers.

19-95: The fromNoise function has been refactored to use the ph module directly for creating instances of Simulation, MaxwellianFluidModel, and ElectromagDiagnostics. This change is in line with the modified import and should be verified to ensure that the ph module provides the same functionality as pyphare.pharein.
#!/bin/bash
# Verify that the 'ph' module provides the same functionality as 'pyphare.pharein'.
rg 'ph.Simulation' --files-with-matches
rg 'ph.MaxwellianFluidModel' --files-with-matches
rg 'ph.ElectromagDiagnostics' --files-with-matches
103-188: The prescribedModes function has undergone similar changes to fromNoise, with the same considerations regarding the use of the ph module and the changes to the diag_options structure.

192-192: The main function now directly runs the simulation returned by fromNoise(). This change seems to be a simplification of the code, which is in line with the user's preference for minimal and efficient code as indicated in the learning.

195-195: The if __name__ == "__main__": guard is a standard Python practice for making the script executable as a standalone program. This change is appropriate and does not affect the functionality.
tests/functional/harris/harris_2d.py (6)

3-3: The import statement has been refactored to use an alias ph for pyphare.pharein, which is consistent with the PR's objective of code formatting and should not affect functionality.

21-37: The config function has been updated to use the ph alias and the simulation parameters have been modified. Verify that these changes to the simulation parameters are intentional and correct.

40-46: The density function has been updated with explicit floating point numbers and calculations. Ensure that these changes are correct and do not introduce any bugs.

124-131: The MaxwellianFluidModel and ElectronModel are now being used with the ph prefix. Verify that these changes are consistent with the rest of the codebase and that the pharein module is the correct location for these classes.

147-152: The get_time function has been updated to use hierarchy_from directly from pyphare.pharesee.hierarchy. Ensure that this change is correct and that the hierarchy_from function is being used properly.

168-172: The main function has been updated to use the Simulator with the updated config function. Verify that the Simulator is being used correctly with the updated config function.
tests/functional/ionIonBeam/ion_ion_beam1d.py (11)
1-11: The import of os has been moved to the top of the file, which is a standard Python convention for better readability and organization.

3-3: The usage of pyphare.pharein has been modified to use the ph alias directly. This change should be verified across the codebase to ensure that all references to pyphare.pharein are consistent with the new alias usage.
#!/bin/bash
# Search for `pyphare.pharein` usage to ensure consistency with the new alias `ph`.
rg 'pyphare.pharein' --vimgrep
25-41: The configuration of the Simulation object has been updated with new parameters and options. Ensure that these changes are intentional and correctly reflect the desired simulation setup.

45-74: The functions densityMain, densityBeam, bx, by, bz, vB, and v0 have been modified to return floating point values directly. This change simplifies the code and is in line with the preference for minimal and efficient code as indicated by PhilipDeegan in a previous learning.

86-91: The MaxwellianFluidModel and ElectronModel objects are now accessed through the ph module directly. This change is consistent with the new alias usage and should be verified for consistency across the codebase.
#!/bin/bash
# Search for `MaxwellianFluidModel` and `ElectronModel` to ensure they are now accessed through the `ph` alias.
rg 'MaxwellianFluidModel|ElectronModel' --vimgrep
98-111: The diagnostics configuration has been updated to use the ph module directly. This change should be verified for consistency and correctness in the context of the simulation's requirements.

116-116: The yaebx function definition remains unchanged and is correctly placed within the file.

139-154: The changes in the growth_b_right_hand function include modifications to the FFT calculations and the curve fitting process. These changes should be carefully reviewed to ensure they maintain the intended functionality and produce correct results.

163-169: The calculation of omega has been updated. Verify that the new formula correctly reflects the intended computation and that the changes do not introduce any errors.

174-276: The main function has been updated with direct instantiation of the Simulator object using the sim object and updated plotting and assertion logic. Ensure that these changes are consistent with the simulation's requirements and that the plotting and assertion logic is correct.

272-272: The assertion checks that the value of gamma is within an acceptable range of the expected value. This is a good practice to ensure the simulation's output is as expected.
tests/functional/shock/shock.py (11)

3-5: The import statements have been modified to use a new module prefix ph. This change is consistent with the PR's objective of improving code formatting and structure.

11-11: The use of mpl.use("Agg") is a configuration setting for matplotlib to use a non-GUI backend, which is suitable for script-based generation of plots. This change seems unrelated to formatting and might be a functional change. Please confirm if this is an intentional addition.

19-40: The configuration function config has been updated with new parameters and comments. The changes are consistent with the PR's objective of enhancing readability and consistency. However, ensure that the new parameters like time_step, final_time, and boundary_types are correctly set and that their values are appropriate for the simulation's requirements.

43-94: The definitions of the density, magnetic field, and velocity functions have been updated with hardcoded values. These changes appear to be more than just formatting; they may affect the simulation's behavior. Please verify that these changes are intended and correctly reflect the desired simulation parameters.

97-98: The MaxwellianFluidModel is instantiated with a new structure, using the ph prefix and passing in the magnetic field and proton parameters. Ensure that this new instantiation is correct and that the parameters passed are consistent with the simulation's requirements.

101-101: The ElectronModel is instantiated with a closure type and temperature. This change seems to be more than just formatting and may affect the simulation's behavior. Please verify that this change is intended and that the temperature value is appropriate.

103-119: The diagnostic configurations have been updated with new prefixes and structure. Ensure that these changes are consistent with the intended use of diagnostics in the simulation and that the timestamps are correctly calculated and used.

130-132: The loop over interp_order and the instantiation of the Simulator with the sim object are consistent with the PR's objective. However, ensure that the changes in the config function are correctly reflected when the Simulator is run.

135-162: The post-processing and visualization code has been updated with new file naming and plotting configurations. Ensure that these changes are intended and that the new file naming convention and plot settings are appropriate for the simulation's output analysis.

162-162: The line ph.global_vars.sim = None seems to be resetting a global variable. This change might be a cleanup step after the simulation runs. Please confirm that this is an intentional change and that it does not interfere with any other parts of the simulation or post-processing.

186-186: The if __name__ == "__main__": guard is a standard Python practice for scripts that are intended to be run as the main program. This change is consistent with the PR's objective of improving code structure.

tests/functional/td/td1d.py (10)

3-3: The import statement has been modified to import pharein as ph, which is consistent with the changes made throughout the file to use the ph alias.

11-11: The usage of mpl.use("Agg") is a configuration setting for matplotlib to use a non-interactive backend, which is appropriate for scripts that will run in environments without a display server (like automated testing or running on a server).

20-35: The configuration of the Simulation object has been updated with new parameters and values. It is important to ensure that these changes are intended and correctly reflect the desired simulation setup.

39-57: The functions for defining physical quantities like density, bx, by, bz, b2, and T have been updated. It's important to ensure that these changes are consistent with the physical model and that the removal of explicit float type casting does not affect the calculations.

90-91: The instantiation of MaxwellianFluidModel has been updated to use the ph alias and the function arguments have been modified. It is crucial to verify that the updated function calls are correct and that the **vvv spread operator is used appropriately to pass the velocity and thermal velocity functions.

94-94: The ElectronModel is instantiated with a closure type and a temperature value. This change should be verified to ensure it aligns with the intended model parameters.

97-97: The calculation of n_dump has been introduced, which seems to be used for determining the number of diagnostic dumps. It is important to verify that this calculation is correct and aligns with the simulation's requirements.

101-112: The instantiation of diagnostics classes ElectromagDiagnostics and FluidDiagnostics has been updated to use the ph alias. The changes should be verified to ensure that the diagnostics are configured correctly and that the timestamps are being set as intended.

117-117: The main function now instantiates a Simulator with the config function and runs the simulation. This change appears to be a straightforward refactoring to start the simulation and should be verified to ensure it functions as expected.

120-121: The addition of the standard Python entry point check is a best practice for making the script executable as a standalone program or importable as a module without running the main function.

tests/functional/tdtagged/td1dtagged.py (8)

4-13: The changes to the import statements and the setting of the matplotlib backend to "Agg" are consistent with formatting and configuration best practices.

19-62: The addition of simple return functions and the reformatting of existing functions appear to be consistent with the goal of improving code readability and do not introduce any logic changes.

91-106: The changes in simulation_params function include formatting and numerical changes. Verify that the updated numerical values (time_step_nbr, time_step, cells, dl) are intended and do not unintentionally alter the simulation's behavior.

126-157: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [109-140]

The changes in the config function include the addition of a new variable assignment and changes in function calls. Ensure that these changes are consistent with the rest of the codebase and do not introduce any functional changes.

143-148: The changes in the withTagging and noRefinement functions appear to be formatting changes and do not introduce any logic changes.

164-291: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [151-250]

The changes in the make_figure function include changes in variable assignments and formatting. Ensure that these changes are purely formatting and do not affect the logic or the output of the function.

269-276: The changes in the post_advance function include changes in function calls and variable assignments. Ensure that these changes are purely formatting and do not affect the logic or the behavior of the function.

279-290: The changes in the main function include changes in import statements, variable formatting, and minor adjustments in mathematical expressions. Ensure that these changes are purely formatting and do not affect the logic or the behavior of the program.

tests/functional/translation/translat1d.py (11)

3-3: The import statement import pyphare.pharein as ph is consistent with the PR's objective to refactor imports for better readability and usage.

10-10: The addition of mpl.use("Agg") sets the matplotlib backend to 'Agg', which is a non-GUI backend suitable for script and server environments. This change is not mentioned in the summary, but it's a good practice if the script is intended to run in environments without a display.

17-37: The config_uni function has been refactored to use the ph alias for pyphare.pharein. The parameters for the Simulation object have been explicitly defined, which improves readability. Ensure that the new parameters match the expected values and types for the Simulation class.

79-83: The MaxwellianFluidModel and ElectronModel are instantiated with the new ph alias and the function definitions have been refactored. Verify that the refactoring has not altered the expected behavior of these models.

88-99: The ElectromagDiagnostics and FluidDiagnostics calls have been refactored to use the ph alias. Verify that the diagnostic options are correctly set and that the timestamps are being generated and used as intended.

105-125: The config_td function has been refactored similarly to config_uni. Verify that the changes are consistent with the intended behavior and that the new parameters are correctly set for the Simulation object.

131-153: New function definitions (S, b2, T) have been added to config_td. Ensure that these functions are correctly implemented and used within the context of the simulation configuration.

182-186: The MaxwellianFluidModel and ElectronModel instantiation in config_td mirrors the changes in config_uni. Verify that the behavior of these models remains consistent with the simulation's requirements.

191-202: The diagnostic calls within config_td have been updated. Verify that the diagnostic options and timestamps are correctly set and used.

208-221: The main function has been refactored to loop over configurations and parameters. Verify that the loop logic and conditional checks are correct and that the simulation runs as expected with the provided parameters.

221-221: Setting ph.global_vars.sim to None after each simulation run is a good practice to ensure that global state does not persist between runs, which could lead to unexpected behavior.

coderabbitai · 2023-12-12T18:26:29Z

tests/core/data/gridlayout/utilities.py

 def removeTupleFormat(str):
-            str = str.replace(",","")
-            str = str.replace("(","")
-            str = str.replace(")","")
-
-            return str
+    str = str.replace(",", "")
+    str = str.replace("(", "")
+    str = str.replace(")", "")

+    return str


The function removeTupleFormat could be further optimized by chaining the replace calls, and the function name should follow the snake_case naming convention as per PEP 8. Additionally, consider renaming the parameter to avoid shadowing the built-in str type.

- def removeTupleFormat(str): + def remove_tuple_format(input_str): - str = str.replace(",", "") - str = str.replace("(", "") - str = str.replace(")", "") + return input_str.replace(",", "").replace("(", "").replace(")", "")

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.

Suggested change

def removeTupleFormat(str):

str = str.replace(",","")

str = str.replace("(","")

str = str.replace(")","")

return str

str = str.replace(",", "")

str = str.replace("(", "")

str = str.replace(")", "")

return str

def remove_tuple_format(input_str):

return input_str.replace(",", "").replace("(", "").replace(")", "")

coderabbitai · 2023-12-12T18:26:30Z

pyphare/pyphare/pharein/examples/job.py

-    compute_every=5,
-    start_teration=0,
-    last_iteration=990
+    diag_type="B", write_every=10, compute_every=5, start_teration=0, last_iteration=990


Typo in parameter name.

- start_teration=0 + start_iteration=0

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.

Suggested change

diag_type="B", write_every=10, compute_every=5, start_teration=0, last_iteration=990

diag_type="B", write_every=10, compute_every=5, start_iteration=0, last_iteration=990

coderabbitai · 2023-12-12T18:26:30Z

pyphare/pyphare/pharein/uniform_model.py

+        self.model_dict.update(
+            {
+                "bx": lambda x: b[0],
+                "by": lambda x: b[1],
+                "bz": lambda x: b[2],
+                "ex": lambda x: e[0],
+                "ey": lambda x: e[1],
+                "ez": lambda x: e[2],
+            }
+        )


The use of lambda functions to return static values in the model_dict is unconventional. While it may be a stylistic choice to ensure all dictionary values are callable, it could introduce unnecessary overhead and complexity. Consider using direct values instead of lambda functions if the values are not intended to be dynamic.

- "bx": lambda x: b[0], - "by": lambda x: b[1], - "bz": lambda x: b[2], - "ex": lambda x: e[0], - "ey": lambda x: e[1], - "ez": lambda x: e[2], + "bx": b[0], + "by": b[1], + "bz": b[2], + "ex": e[0], + "ey": e[1], + "ez": e[2],

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.

Suggested change

self.model_dict.update(

{

"bx": lambda x: b[0],

"by": lambda x: b[1],

"bz": lambda x: b[2],

"ex": lambda x: e[0],

"ey": lambda x: e[1],

"ez": lambda x: e[2],

}

)

self.model_dict.update(

{

"bx": b[0],

"by": b[1],

"bz": b[2],

"ex": e[0],

"ey": e[1],

"ez": e[2],

}

)

coderabbitai · 2023-12-12T18:26:30Z

tests/diagnostic/__init__.py

 def timestamps_with_step(sim, dump_step):
    import numpy as np
+
    nbr_dump_step = sim.final_time / dump_step


The calculation of nbr_dump_step has been changed from using dump_step to sim.final_time. This is not just a formatting change but alters the behavior of the function. Confirm that this change is intentional and correctly implements the desired logic.

coderabbitai · 2023-12-12T18:26:30Z

tests/diagnostic/__init__.py

 def all_timestamps(sim):
    import numpy as np
+
    nbr_dump_step = int(sim.final_time / sim.time_step) + 1


The calculation of nbr_dump_step has been changed from using sim.final_time to sim.time_step. This is not just a formatting change but alters the behavior of the function. Confirm that this change is intentional and correctly implements the desired logic.

coderabbitai · 2023-12-12T18:26:33Z

pyphare/pyphare/pharesee/geometry.py

            continue

        for pd_key, info_list in gaboxes[ilvl].items():
-
            # if periodic, always true for now
-            if True:  #lgtm [py/constant-conditional-expression]
+            if True:  # lgtm [py/constant-conditional-expression]
                refined_domain_box = hierarchy.refined_domain_box(ilvl)
                n_ghosts = lvl.patches[0].patch_datas[pd_key].ghosts_nbr
                patches = get_periodic_list(lvl.patches, refined_domain_box, n_ghosts)

            for info in info_list:
-
                patch_data, ghostAreaBoxes = info["pdata"], info["boxes"]

-                check_patches = [p for p in patches if p.patch_datas[pd_key] is not patch_data]
+                check_patches = [
+                    p for p in patches if p.patch_datas[pd_key] is not patch_data
+                ]

                if len(check_patches) == 0:
                    check_patches = patches

                for gabox in ghostAreaBoxes:
-
                    remaining = gabox - check_patches[0].box

                    for patch in check_patches[1:]:


Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [453-496]

The conditional check if True: is flagged by the existing comment from github-advanced-security[bot]. This should be corrected to reflect the actual condition or removed if it's not needed.

- if True: # lgtm [py/constant-conditional-expression] + if some_condition: # Replace 'True' with the actual condition or remove if unnecessary

coderabbitai · 2023-12-12T18:26:33Z

pyphare/pyphare/pharesee/plotting.py

    else:
        ax = kwargs["ax"]
        fig = ax.figure
-    axis = kwargs.get("axis",("Vx","Vy"))
-    vaxis = {"Vx":0, "Vy":1, "Vz":2}
+    axis = kwargs.get("axis", ("Vx", "Vy"))
+    vaxis = {"Vx": 0, "Vy": 1, "Vz": 2}

    if axis[0] in vaxis:
-        x = particles.v[:,vaxis[axis[0]]]
+        x = particles.v[:, vaxis[axis[0]]]
    elif axis[0] == "x":
        x = particles.x
    if axis[1] in vaxis:
-        y = particles.v[:,vaxis[axis[1]]]
+        y = particles.v[:, vaxis[axis[1]]]

-    bins = kwargs.get("bins", (50,50))
-    h, xh, yh  = np.histogram2d(x, y,
-              bins=kwargs.get("bins", bins),
-              weights=particles.weights[:,0])
+    bins = kwargs.get("bins", (50, 50))
+    h, xh, yh = np.histogram2d(
+        x, y, bins=kwargs.get("bins", bins), weights=particles.weights[:, 0]
+    )

-    if "gaussian_filter_sigma" in kwargs and "median_filter_size" not in kwargs: 
+    if "gaussian_filter_sigma" in kwargs and "median_filter_size" not in kwargs:
        from scipy.ndimage import gaussian_filter
-        sig = kwargs.get("gaussian_filter_sigma", (0,0))
+
+        sig = kwargs.get("gaussian_filter_sigma", (0, 0))
        image = gaussian_filter(h.T, sigma=sig)
-    elif "median_filter_size" in kwargs and "gaussian_filter_sigma" not in kwargs: 
+    elif "median_filter_size" in kwargs and "gaussian_filter_sigma" not in kwargs:
        from scipy.ndimage import median_filter
-        siz = kwargs.get("median_filter_size", (0,0))
+
+        siz = kwargs.get("median_filter_size", (0, 0))
        image = median_filter(h.T, size=siz)
    elif "gaussian_filter_sigma" not in kwargs and "median_filter_size" not in kwargs:
        image = h.T
    else:
-        raise ValueError("gaussian and median filters can not be called at the same time")
+        raise ValueError(
+            "gaussian and median filters can not be called at the same time"
+        )

    cmap = kwargs.get("cmap", "jet")



Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [50-145]

Considering the user's preference for minimal and efficient code, it may be worth reviewing the changes in the dist_plot function to ensure that any redundant or inefficient code has been removed or optimized.

coderabbitai · 2023-12-12T18:26:33Z

pyphare/pyphare/pharesee/particles.py

+        y1 = particles.iCells[:, 1] + particles.deltas[:, 1]
+        return np.argsort(np.sqrt((x1**2 + y1**2)) / (x1 / y1))


The bot has flagged a potential issue with the local variable 'y1' being used before it is initialized. Verify that this issue has been addressed in the current changes.

coderabbitai · 2023-12-12T18:26:33Z

pyphare/pyphare/pharein/maxwellian_fluid_model.py

    def validate2d(self, sim, atol):
-
-        domain_box = Box([0]*sim.ndim, sim.cells)
+        domain_box = Box([0] * sim.ndim, sim.cells)
        layout = GridLayout(domain_box, sim.origin, sim.dl, sim.interp_order)
-        nbrDualGhosts= layout.nbrGhostsPrimal(sim.interp_order)
-        nbrPrimalGhosts= layout.nbrGhostsPrimal(sim.interp_order)
-        directions=["X", "Y"]
+        nbrDualGhosts = layout.nbrGhostsPrimal(sim.interp_order)
+        nbrPrimalGhosts = layout.nbrGhostsPrimal(sim.interp_order)
+        directions = ["X", "Y"]
        domain = sim.simulation_domain()
        bx = self.model_dict["bx"]
        by = self.model_dict["by"]
        bz = self.model_dict["bz"]
        is_periodic = True
        not_periodic = []

-        def getCoord(L,R, idir):
+        def getCoord(L, R, idir):
            if idir == 0:
-                return (L,np.zeros_like(L)), (R,np.zeros_like(R))
+                return (L, np.zeros_like(L)), (R, np.zeros_like(R))
            else:
-                return (np.zeros_like(L),L), (np.zeros_like(R),R)
+                return (np.zeros_like(L), L), (np.zeros_like(R), R)

        print("2d periodic validation")
        for idir in np.arange(sim.ndim):
            print("validating direction ...", idir)
            if sim.boundary_types[idir] == "periodic":
                print(f"direction {idir} is periodic?")
-                dual_left = (np.arange(-nbrDualGhosts, nbrDualGhosts)+0.5)*sim.dl[0]
+                dual_left = (np.arange(-nbrDualGhosts, nbrDualGhosts) + 0.5) * sim.dl[0]
                dual_right = dual_left + domain[0]
-                primal_left = np.arange(-nbrPrimalGhosts, nbrPrimalGhosts)*sim.dl[0]
+                primal_left = np.arange(-nbrPrimalGhosts, nbrPrimalGhosts) * sim.dl[0]
                primal_right = primal_left + domain[0]

                direction = directions[idir]

-                for b_i,b_name in zip((bx,by,bz), ("Bx", "By", "Bz")):
+                for b_i, b_name in zip((bx, by, bz), ("Bx", "By", "Bz")):
                    if layout.qtyIsDual(b_name, direction):
-                        L,R = dual_left, dual_right
+                        L, R = dual_left, dual_right
                    else:
-                        L,R = primal_left, primal_right
+                        L, R = primal_left, primal_right

                coordsL, coordsR = getCoord(L, R, idir)
-                check  = np.allclose(b_i(*coordsL), b_i(*coordsR),atol=atol, rtol=0)
+                check = np.allclose(b_i(*coordsL), b_i(*coordsR), atol=atol, rtol=0)
                if not check:
-                    not_periodic += [(b_name,idir)]
+                    not_periodic += [(b_name, idir)]
                is_periodic &= check

                for pop in self.populations:
                    functions = ("vx", "vy", "vz", "vthx", "vthy", "vthz")
-                    L,R = primal_left, primal_right
-                    coordsL, coordsR = getCoord(L,R, idir)
+                    L, R = primal_left, primal_right
+                    coordsL, coordsR = getCoord(L, R, idir)

                    for fn in functions:
                        f = self.model_dict[pop][fn]
                        fL = f(*coordsL)
                        fR = f(*coordsR)
-                        check = np.allclose(fL,fR, atol=atol, rtol=0)
-                        print(f"checked {fn} : fL = {fL} and fR = {fR} and check = {check}")
+                        check = np.allclose(fL, fR, atol=atol, rtol=0)
+                        print(
+                            f"checked {fn} : fL = {fL} and fR = {fR} and check = {check}"
+                        )
                        if not check:
-                            not_periodic += [(fn,idir)]
-                        is_periodic &=check
+                            not_periodic += [(fn, idir)]
+                        is_periodic &= check

        if not is_periodic:
-            print("Warning: Simulation is periodic but some functions are not : ", not_periodic)
+            print(
+                "Warning: Simulation is periodic but some functions are not : ",
+                not_periodic,
+            )
            if sim.strict:
                raise RuntimeError("Simulation is not periodic")


The validate2d method has been reformatted, and additional print statements have been added for debugging purposes. The changes are consistent with the PR's objective to improve formatting and readability. However, the same potential issue as in validate1d is present here with the np.allclose function comparing the same values to themselves. This should be reviewed and corrected if necessary.

- np.allclose(b_i(*coordsL), b_i(*coordsL), atol=atol, rtol=0) + np.allclose(b_i(*coordsL), b_i(*coordsR), atol=atol, rtol=0)

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.

Suggested change

def validate2d(self, sim, atol):

domain_box = Box([0]*sim.ndim, sim.cells)

domain_box = Box([0] * sim.ndim, sim.cells)

layout = GridLayout(domain_box, sim.origin, sim.dl, sim.interp_order)

nbrDualGhosts= layout.nbrGhostsPrimal(sim.interp_order)

nbrPrimalGhosts= layout.nbrGhostsPrimal(sim.interp_order)

directions=["X", "Y"]

nbrDualGhosts = layout.nbrGhostsPrimal(sim.interp_order)

nbrPrimalGhosts = layout.nbrGhostsPrimal(sim.interp_order)

directions = ["X", "Y"]

domain = sim.simulation_domain()

bx = self.model_dict["bx"]

by = self.model_dict["by"]

bz = self.model_dict["bz"]

is_periodic = True

not_periodic = []

def getCoord(L,R, idir):

def getCoord(L, R, idir):

if idir == 0:

return (L,np.zeros_like(L)), (R,np.zeros_like(R))

return (L, np.zeros_like(L)), (R, np.zeros_like(R))

else:

return (np.zeros_like(L),L), (np.zeros_like(R),R)

return (np.zeros_like(L), L), (np.zeros_like(R), R)

print("2d periodic validation")

for idir in np.arange(sim.ndim):

print("validating direction ...", idir)

if sim.boundary_types[idir] == "periodic":

print(f"direction {idir} is periodic?")

dual_left = (np.arange(-nbrDualGhosts, nbrDualGhosts)+0.5)*sim.dl[0]

dual_left = (np.arange(-nbrDualGhosts, nbrDualGhosts) + 0.5) * sim.dl[0]

dual_right = dual_left + domain[0]

primal_left = np.arange(-nbrPrimalGhosts, nbrPrimalGhosts)*sim.dl[0]

primal_left = np.arange(-nbrPrimalGhosts, nbrPrimalGhosts) * sim.dl[0]

primal_right = primal_left + domain[0]

direction = directions[idir]

for b_i,b_name in zip((bx,by,bz), ("Bx", "By", "Bz")):

for b_i, b_name in zip((bx, by, bz), ("Bx", "By", "Bz")):

if layout.qtyIsDual(b_name, direction):

L,R = dual_left, dual_right

L, R = dual_left, dual_right

else:

L,R = primal_left, primal_right

L, R = primal_left, primal_right

coordsL, coordsR = getCoord(L, R, idir)

check = np.allclose(b_i(*coordsL), b_i(*coordsR),atol=atol, rtol=0)

check = np.allclose(b_i(*coordsL), b_i(*coordsR), atol=atol, rtol=0)

if not check:

not_periodic += [(b_name,idir)]

not_periodic += [(b_name, idir)]

is_periodic &= check

for pop in self.populations:

functions = ("vx", "vy", "vz", "vthx", "vthy", "vthz")

L,R = primal_left, primal_right

coordsL, coordsR = getCoord(L,R, idir)

L, R = primal_left, primal_right

coordsL, coordsR = getCoord(L, R, idir)

for fn in functions:

f = self.model_dict[pop][fn]

fL = f(*coordsL)

fR = f(*coordsR)

check = np.allclose(fL,fR, atol=atol, rtol=0)

print(f"checked {fn} : fL = {fL} and fR = {fR} and check = {check}")

check = np.allclose(fL, fR, atol=atol, rtol=0)

print(

f"checked {fn} : fL = {fL} and fR = {fR} and check = {check}"

)

if not check:

not_periodic += [(fn,idir)]

is_periodic &=check

not_periodic += [(fn, idir)]

is_periodic &= check

if not is_periodic:

print("Warning: Simulation is periodic but some functions are not : ", not_periodic)

print(

"Warning: Simulation is periodic but some functions are not : ",

not_periodic,

)

if sim.strict:

raise RuntimeError("Simulation is not periodic")

def validate2d(self, sim, atol):

domain_box = Box([0] * sim.ndim, sim.cells)

layout = GridLayout(domain_box, sim.origin, sim.dl, sim.interp_order)

nbrDualGhosts = layout.nbrGhostsPrimal(sim.interp_order)

nbrPrimalGhosts = layout.nbrGhostsPrimal(sim.interp_order)

directions = ["X", "Y"]

domain = sim.simulation_domain()

bx = self.model_dict["bx"]

by = self.model_dict["by"]

bz = self.model_dict["bz"]

is_periodic = True

not_periodic = []

def getCoord(L, R, idir):

if idir == 0:

return (L, np.zeros_like(L)), (R, np.zeros_like(R))

else:

return (np.zeros_like(L), L), (np.zeros_like(R), R)

print("2d periodic validation")

for idir in np.arange(sim.ndim):

print("validating direction ...", idir)

if sim.boundary_types[idir] == "periodic":

print(f"direction {idir} is periodic?")

dual_left = (np.arange(-nbrDualGhosts, nbrDualGhosts) + 0.5) * sim.dl[0]

dual_right = dual_left + domain[0]

primal_left = np.arange(-nbrPrimalGhosts, nbrPrimalGhosts) * sim.dl[0]

primal_right = primal_left + domain[0]

direction = directions[idir]

for b_i, b_name in zip((bx, by, bz), ("Bx", "By", "Bz")):

if layout.qtyIsDual(b_name, direction):

L, R = dual_left, dual_right

else:

L, R = primal_left, primal_right

coordsL, coordsR = getCoord(L, R, idir)

check = np.allclose(b_i(*coordsL), b_i(*coordsR), atol=atol, rtol=0)

if not check:

not_periodic += [(b_name, idir)]

is_periodic &= check

for pop in self.populations:

functions = ("vx", "vy", "vz", "vthx", "vthy", "vthz")

L, R = primal_left, primal_right

coordsL, coordsR = getCoord(L, R, idir)

for fn in functions:

f = self.model_dict[pop][fn]

fL = f(*coordsL)

fR = f(*coordsR)

check = np.allclose(fL, fR, atol=atol, rtol=0)

print(

f"checked {fn} : fL = {fL} and fR = {fR} and check = {check}"

)

if not check:

not_periodic += [(fn, idir)]

is_periodic &= check

if not is_periodic:

print(

"Warning: Simulation is periodic but some functions are not : ",

not_periodic,

)

if sim.strict:

raise RuntimeError("Simulation is not periodic")

coderabbitai · 2023-12-12T18:26:33Z

pyphare/pyphare/pharein/simulation.py

+    dom_and_dl = "domain_size" in kwargs and "dl" in kwargs and "cells" not in kwargs
+    cells_and_dl = "cells" in kwargs and "dl" in kwargs and "domain_size" not in kwargs
+    dom_and_cells = "cells" in kwargs and "domain_size" in kwargs and "dl" not in kwargs

    if not (dom_and_dl or dom_and_cells or cells_and_dl):
-        raise ValueError("Error: Specify either 'domain_size' and 'dl' or 'cells' and 'dl' or 'domain_size' and 'dl'")
+        raise ValueError(
+            "Error: Specify either 'domain_size' and 'dl' or 'cells' and 'dl' or 'domain_size' and 'dl'"
+        )


The error message in the ValueError is not clear. It should specify that only two of the three parameters ('domain_size', 'dl', 'cells') should be provided, not all three.

- "Error: Specify either 'domain_size' and 'dl' or 'cells' and 'dl' or 'domain_size' and 'dl'" + "Error: Specify exactly two parameters out of 'domain_size', 'dl', and 'cells'"

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.

Suggested change

dom_and_dl = "domain_size" in kwargs and "dl" in kwargs and "cells" not in kwargs

cells_and_dl = "cells" in kwargs and "dl" in kwargs and "domain_size" not in kwargs

dom_and_cells = "cells" in kwargs and "domain_size" in kwargs and "dl" not in kwargs

if not (dom_and_dl or dom_and_cells or cells_and_dl):

raise ValueError("Error: Specify either 'domain_size' and 'dl' or 'cells' and 'dl' or 'domain_size' and 'dl'")

raise ValueError(

"Error: Specify either 'domain_size' and 'dl' or 'cells' and 'dl' or 'domain_size' and 'dl'"

)

dom_and_dl = "domain_size" in kwargs and "dl" in kwargs and "cells" not in kwargs

cells_and_dl = "cells" in kwargs and "dl" in kwargs and "domain_size" not in kwargs

dom_and_cells = "cells" in kwargs and "domain_size" in kwargs and "dl" not in kwargs

if not (dom_and_dl or dom_and_cells or cells_and_dl):

raise ValueError(

"Error: Specify exactly two parameters out of 'domain_size', 'dl', and 'cells'"

)

PhilipDeegan · 2023-12-13T09:30:11Z

functional tests passing on my own build
https://hephaistos.lpp.polytechnique.fr/teamcity/buildConfiguration/Phare_Phare_Users_Phil_Build/79830?buildTab=artifacts#%2Ffunctional.mp4.zip;%2Ffunctional.png.zip!%2Ftdtagged

blacking

de6bee3

github-advanced-security bot found potential problems Dec 10, 2023

View reviewed changes

PhilipDeegan requested a review from nicolasaunai December 12, 2023 10:42

PhilipDeegan force-pushed the blacking branch from 08d4641 to b2ac941 Compare December 12, 2023 10:47

imports/static sim cleanup

cf1fcff

PhilipDeegan force-pushed the blacking branch from b2ac941 to cf1fcff Compare December 12, 2023 18:07

coderabbitai bot reviewed Dec 12, 2023

View reviewed changes

nicolasaunai approved these changes Dec 14, 2023

View reviewed changes

nicolasaunai merged commit 7948fab into PHAREHUB:master Dec 14, 2023
9 of 10 checks passed

PhilipDeegan deleted the blacking branch December 14, 2023 13:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

formatting python via black #792

formatting python via black #792

PhilipDeegan commented Dec 10, 2023 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 10, 2023 •

edited

Loading

Walkthrough

Changes

Chat with CodeRabbit Bot (`@coderabbitai`)

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

PhilipDeegan commented Dec 12, 2023

coderabbitai bot left a comment

coderabbitai bot Dec 12, 2023

coderabbitai bot Dec 12, 2023

coderabbitai bot Dec 12, 2023

coderabbitai bot Dec 12, 2023

coderabbitai bot Dec 12, 2023

coderabbitai bot Dec 12, 2023

coderabbitai bot Dec 12, 2023

coderabbitai bot Dec 12, 2023

coderabbitai bot Dec 12, 2023

coderabbitai bot Dec 12, 2023

PhilipDeegan commented Dec 13, 2023



		z1 = particles.iCells[:, 2] + particles.deltas[:, 2]
		return np.argsort(np.sqrt((x12 + y12 + z1**2)) / (x1 / y1 / z1))

	diag_type="B", write_every=10, compute_every=5, start_teration=0, last_iteration=990
	diag_type="B", write_every=10, compute_every=5, start_iteration=0, last_iteration=990

		y1 = particles.iCells[:, 1] + particles.deltas[:, 1]
		return np.argsort(np.sqrt((x12 + y12)) / (x1 / y1))

formatting python via black #792

formatting python via black #792

Conversation

PhilipDeegan commented Dec 10, 2023 • edited by coderabbitai bot Loading

Summary by CodeRabbit

coderabbitai bot commented Dec 10, 2023 • edited Loading

Walkthrough

Changes

Chat with CodeRabbit Bot (@coderabbitai)

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

PhilipDeegan commented Dec 12, 2023

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot Dec 12, 2023

Choose a reason for hiding this comment

coderabbitai bot Dec 12, 2023

Choose a reason for hiding this comment

coderabbitai bot Dec 12, 2023

Choose a reason for hiding this comment

coderabbitai bot Dec 12, 2023

Choose a reason for hiding this comment

coderabbitai bot Dec 12, 2023

Choose a reason for hiding this comment

coderabbitai bot Dec 12, 2023

Choose a reason for hiding this comment

coderabbitai bot Dec 12, 2023

Choose a reason for hiding this comment

coderabbitai bot Dec 12, 2023

Choose a reason for hiding this comment

coderabbitai bot Dec 12, 2023

Choose a reason for hiding this comment

coderabbitai bot Dec 12, 2023

Choose a reason for hiding this comment

PhilipDeegan commented Dec 13, 2023

PhilipDeegan commented Dec 10, 2023 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 10, 2023 •

edited

Loading

Chat with CodeRabbit Bot (`@coderabbitai`)

CodeRabbit Configration File (`.coderabbit.yaml`)