Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Feature/Update validation for columnar #734

Merged
merged 6 commits into from
Sep 25, 2024
Merged

Feature/Update validation for columnar #734

merged 6 commits into from
Sep 25, 2024

Conversation

figueroa1395
Copy link
Contributor

@figueroa1395 figueroa1395 commented Sep 25, 2024 •
edited
Loading

Update validation functions to support columnar data:

  • Update validate_input_data function.
  • Update validate_batch_data function.
  • Update tests:
    • test_input_validation.py
    • test_batch_validation.py

@figueroa1395
added columnar data validation
e951ec3 
Signed-off-by: Santiago Figueroa Manrique <santiago.figueroa.manrique@alliander.com>
@figueroa1395
input validation tests
9f92e6c 
Signed-off-by: Santiago Figueroa Manrique <santiago.figueroa.manrique@alliander.com>
@figueroa1395
batch validation tests - failing
b90bd67 
Signed-off-by: Santiago Figueroa Manrique <santiago.figueroa.manrique@alliander.com>
@figueroa1395
batch validation tests pass - temporary
9ff6326 
Signed-off-by: Santiago Figueroa Manrique <santiago.figueroa.manrique@alliander.com>
@figueroa1395
Copy link
Contributor Author

At this moment, the columnar based batch data is validated by converting the full batch to row based, which may have a big memory overhead. However, once #699 is finished, this will be upgraded to do the conversion per individual batches.

@figueroa1395
Formatting
291b134 
Signed-off-by: Santiago Figueroa Manrique <santiago.figueroa.manrique@alliander.com>
mgovers
mgovers reviewed Sep 25, 2024
View reviewed changes
src/power_grid_model/_utils.py Outdated Show resolved Hide resolved
src/power_grid_model/validation/validation.py Outdated Show resolved Hide resolved
tests/unit/validation/test_batch_validation.py Outdated Show resolved Hide resolved
tests/unit/validation/test_input_validation.py Outdated Show resolved Hide resolved
@figueroa1395
Addressed comments
48434b7 
Signed-off-by: Santiago Figueroa Manrique <santiago.figueroa.manrique@alliander.com>
mgovers
mgovers approved these changes Sep 25, 2024
View reviewed changes
Copy link
Member

@mgovers mgovers left a comment

Choose a reason for hiding this comment

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

since validation for columnar data is not blocked by the Python API (since we already have preliminary python support), it's also OK to merge this. your call

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@figueroa1395 figueroa1395 marked this pull request as ready for review September 25, 2024 07:45
@figueroa1395
Copy link
Contributor Author

Merging now and doing the incremental upgrade to per-batch scenario validation later. This way the functionality is already provided to the users.

@figueroa1395 figueroa1395 added this pull request to the merge queue Sep 25, 2024
Merged via the queue into main with commit 850ef79 Sep 25, 2024
26 checks passed
@figueroa1395 figueroa1395 deleted the feature/update_validation_for_columnar branch September 25, 2024 08:59
This was referenced Sep 30, 2024
Closed
Merged
@mgovers mgovers mentioned this pull request Nov 5, 2024
Closed
27 tasks
@figueroa1395 figueroa1395 mentioned this pull request Nov 7, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mgovers mgovers mgovers approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@figueroa1395 @mgovers