-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
update add_field_to function for improved error handling #696
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dtrai2
force-pushed
the
dev-revise-add-field-to-error-handling
branch
from
November 11, 2024 07:54
53b7ff9
to
2fbfa7e
Compare
ekneg54
requested changes
Nov 11, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you for your contribution. please have a look on my remarks.
dtrai2
force-pushed
the
dev-revise-add-field-to-error-handling
branch
from
November 12, 2024 09:23
032a95c
to
4de86e2
Compare
- Add raise_on_failure parameter to raise exceptions on failure. - Refactor function for clarity and consistency. - Update unit tests to reflect new exception handling logic.
- Catch and handle FieldExistsWarning to raise CriticalInputError.
- Add `add_batch_to` method - Replaced `output_field` with `target_field` for consistency - Improved exception handling and reduced redundant code
- one test is still broken, needs further investigation why
- Refactored helper functions to consistently raise exceptions. - Improved error handling by eliminating silent failures. - Updated related unit tests to expect raised exceptions. - highlight breaking change in CHANGELOG.md
- Renames `overwrite_output_field` to `overwrite_target_field` in multiple files. - Ensures consistency in method signatures
- Ensure document state is asserted correctly after exceptions are raised. - Update test name and add a comment for clarity.
- Merges target and content arguments into a single field argument.
- Replace `add_batch_to` with `add_field_to` throughout code. - Update helper functions to streamline `add_field_to` usage.
- Add tests to cover adding multiple fields at once. - Include scenarios for overwriting, extending lists, and raising warnings.
- Corrected 'delimiter' typo across test and implementation files. - Updated changelog to reflect this fix.
dtrai2
force-pushed
the
dev-revise-add-field-to-error-handling
branch
from
November 13, 2024 10:46
4d5dbf8
to
827b7f4
Compare
- Simplified function name to better reflect its purpose. - Updated all instances where the function is invoked to maintain consistency.
ekneg54
approved these changes
Nov 14, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
add_field_to
toadd_fields_to
add_fields_to
always raisesFieldExistsWarning
on failureadd_fields_to
accepts a batch of fields in form of a dictadd_fields_to
can take a Rule as argument now, needed to raise properFieldExistsWarning
; is optional because of use ofadd_fields_to
outside from processors, e.g. InputConnectorsadd_fields_to
function for clarity and consistency.add_fields_to
(especially processors)ToDo:
add_fields_to
method_write_target_field
inabc/processor.py