Conversation
Tests passing
Tests passing
…pe-schema into feature/debug-calculate-values
…ss_binary_ops, respectively
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #109 +/- ##
==========================================
+ Coverage 87.20% 87.25% +0.04%
==========================================
Files 11 11
Lines 3361 3373 +12
==========================================
+ Hits 2931 2943 +12
Misses 430 430 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Converted to draft. I added a couple of more tests to increase the CodeCov, but they're on the server that's down. Can recreate them depending on how long that takes to fix. |
Adding test case for empty unary operations (for codecov) Removing files generated in tests
| ) | ||
| else: | ||
| relevant_data = processed_relevant_data | ||
|
|
There was a problem hiding this comment.
This change was added because when using tag_to_var_map, relevant_data wouldn't already be a np array. Can think about if there are better ways to handle this
There was a problem hiding this comment.
This seems like a reasonable solution to me!
pype_schema/tests/test_operations.py
Outdated
| tag.calculate_values(data) | ||
| elif data_type == "InvalidUnary": | ||
| data = "invalid_data_type" # pass a string | ||
| tag.calculate_values(data) |
There was a problem hiding this comment.
Alternatively could pass in a string for the "Invalid" test too and make them all use the same invalid data type?
| else: | ||
| raise TypeError("Data must be either a list, array, or Series") | ||
| if result is None: | ||
| raise TypeError("Data must be either a list, array, or Series") |
There was a problem hiding this comment.
Moved this TypeError to the end to simplify when this is raised and help with codecov... alternatively can just add separate tests for each operation type
There was a problem hiding this comment.
Does this work? I would think that there would be an UnboundLocalError because the variable result may be referenced without being defined.
Besides that bug, I'm all for streamlining the logic
There was a problem hiding this comment.
Good catch!! That did raise an UnboundLocalError.
The test in test_calculate_values was passing because a TypeError was being raised earlier, from process_unary_ops in tag.py. So unary_helper was never even called
I removed that type checking entirely instead, since unary_helper shouldn't ever be called outside process_unary_ops... is that ok or do you prefer to keep it as it was originally just in case?
fletchapin
left a comment
fletchapin
left a comment
There was a problem hiding this comment.
I believe there is a potential UnboundLocalError, but if that is addressed then we're good to go!
| else: | ||
| raise TypeError("Data must be either a list, array, or Series") | ||
| if result is None: | ||
| raise TypeError("Data must be either a list, array, or Series") |
There was a problem hiding this comment.
Does this work? I would think that there would be an UnboundLocalError because the variable result may be referenced without being defined.
Besides that bug, I'm all for streamlining the logic
| ) | ||
| else: | ||
| relevant_data = processed_relevant_data | ||
|
|
There was a problem hiding this comment.
This seems like a reasonable solution to me!
…n process_unary_ops
Adding tests for: - TypeError from unary_helper - tag_to_var_map with List data - invalid mode in VirtualTag class
2 participants
Resolves #107