Skip to content

Conversation

pabloitu
Copy link
Collaborator

@pabloitu pabloitu commented Mar 27, 2025

The magnitude attribute it is now an argument of the CartesianRegion2D class, and not left to the user to hardcode it after class instantiation. It is optional (defaults to None), in case the region is used exclusively in the spatial sense.

fix: Added magnitudes as init attribute for CartesianGrid2D class. Defaults to none. Changed default CSEP regions instantiation to account for this, instead of hardcoding magnitudes.

tests: added magnitude instantiation in class unit tests.

closes #257

pyCSEP Pull Request Checklist

Please check out the contributing guidelines for some tips
on making pull requests to pyCSEP.

Fixes issue #(please fill in or delete if not needed).

Type of change:

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@pabloitu pabloitu requested a review from Copilot March 27, 2025 14:58
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a new optional "magnitudes" attribute to the CartesianGrid2D class and updates related instantiations across the code to use this parameter instead of assigning it after initialization. Key changes include:

  • Updating the CartesianGrid2D constructor to accept an optional magnitudes parameter.
  • Modifying region-builder functions in csep/core/regions.py to pass magnitudes directly.
  • Adjusting unit tests in tests/test_spatial.py to include a magnitudes array during grid instantiation.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/test_spatial.py Updated grid instantiation to include magnitudes in unit tests
csep/core/regions.py Refactored region creation functions to pass magnitudes

@pabloitu pabloitu requested a review from Serra314 March 27, 2025 14:58
…faults to none. Changed default CSEP regions instantiation to account for this, instead of hardcoding magnitudes.

tests: added magnitude instantiation in class unit tests.
@pabloitu pabloitu force-pushed the 257-cartesiangrid2d-not-having-magnitude-attribute-instantiation branch from bee4da4 to 15ad8df Compare April 3, 2025 15:05
@Serra314 Serra314 merged commit eb10890 into SCECcode:main Apr 7, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CartesianGrid2D not having magnitude attribute instantiation
2 participants