[chore]Make get_field_attr safe and improve control flow in ts_core.py #319
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
Refactor get_field_attr in library.py: - Ensure safety using list slicing to avoid IndexError. - Use .get() for dictionary access to handle default cases and prevent KeyError. Improve control flow in ts_core.py: - Remove nesting by exiting early in match_conditions. - Eliminate the need for try-catch blocks for better readability and control flow. Note: Kept existing_fields case unchanged due to add_field_to_entry's procedural nature. this commit introduces changes to library.py get_field_attr
cleans whitespace from file
Fix get_field_attr in library.py to ensure proper handling of empty keys list: - Previously, slicing avoided IndexError but returned a list instead. - Ensure the value is read only if present, defaulting to -1 otherwise. Ideal scenario: Define this function to return an int, as expected by the software elsewhere.
keep modified version only. This change ensures that the attribute value is correctly fetched using the _ensure_field dictionary, maintaining expected functionality.
|
FYI, in the future you don't need to close and open a new PR when you want to add new commits. You can just (sometimes, force) push to the same remote branch |
|
@seakrueger I know, but I didn't know if id find what was the introduced issue on time, so to prevent people wasting time taking a look I closed it and created a new one when ready. cheers |
CyanVoxel
added
Type: Refactor
eivl
approved these changes
Sep 6, 2024
| ] | ||
| match attribute.lower(): | ||
| case "id": | ||
| return val[0] if not any(val := list(entry_field.keys())[:1]) else -1 |
There was a problem hiding this comment.
What is the -1 used for? this looked at first glance as a rewrite to structural pattern matching, are there anything else?
the dense nature of the code makes it hard to read compared to list(entry_field.keys())[0]
|
I'm afraid this PR is now obsolete when #332 has been merged. If you disagree, feel free to reopen it. |
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.
Refactor get_field_attr in library.py and improve control flow in ts_core.py
Changes:
1. Refactor
get_field_attrinlibrary.py:IndexError.anyto determine if there is a value before using it..get()for dictionary access to handle default cases and preventKeyError.2. Improve control flow in
ts_core.py:match_conditions.Additional Fix:
Note:
existing_fieldscase unchanged due toadd_field_to_entry's procedural nature.