Remove unused vector to/from native conversion functions #70
Conversation
robsdedude
force-pushed
the
clean-up/remove-vector-native-conversion-functions
branch
2 times, most recently
from
745a6aa to
ae8a2c0
Compare
robsdedude
force-pushed
the
clean-up/remove-vector-native-conversion-functions
branch
2 times, most recently
from
410d93e to
80b57c9
Compare
robsdedude
force-pushed
the
clean-up/remove-vector-native-conversion-functions
branch
from
80b57c9 to
b1663fb
Compare
RichardIrons-neo4j
left a comment
RichardIrons-neo4j
left a comment
There was a problem hiding this comment.
Only one note. Up to you if you bother with it.
🦩
| let len = bytes.len(); | ||
| if len % type_size != 0 { | ||
| return Err(PyErr::new::<PyValueError, _>( | ||
| "Data length not a multiple of type_size", |
There was a problem hiding this comment.
Could add the bad length and size to the message here?
There was a problem hiding this comment.
Could do. But this is really just a second line of defense. The driver code calling this already preforms this check.
Assignment here: https://github.com/neo4j/neo4j-python-driver/blob/d10d2cd689340d9f2acfe67ca19131936e722f04/src/neo4j/vector.py#L618-L623
Check followed by call here: https://github.com/neo4j/neo4j-python-driver/blob/d10d2cd689340d9f2acfe67ca19131936e722f04/src/neo4j/vector.py#L581-L585
So I think it's not worth bothering. I also remove it and let if fall through to the assert_eq!(src.len() % N, 0); in swap_n resulting in an uglier error message. But again, only when messing up in the driver code would this ever surface. 🤷 I really don't have strong opinion on this.
robsdedude
deleted the
clean-up/remove-vector-native-conversion-functions
branch
This error path shouldn't ever be hit if the driver is working as intended. This change might make debugging while developing the driver slightly easier in the future. Initial discussion: #70 (comment)
This error path shouldn't ever be hit if the driver is working as intended. This change might make debugging while developing the driver slightly easier in the future. Initial discussion: #70 (comment)
This error path shouldn't ever be hit if the driver is working as intended. This change might make debugging while developing the driver slightly easier in the future. Initial discussion: #70 (comment)
Closes: DRIVERS-163
Blocked by: