fix for bug with field-types indices not being correctly referenced #11
Conversation
| for num, field in enumerate(line[self.prop_offset:]): | ||
| field_type_idx = self.prop_offset+num | ||
| try : | ||
| FIELD_TYPES[self.entity_str][num] |
There was a problem hiding this comment.
For consistency, let's change this line to index into field_type_idx rather than num.
jeffreylovitz
left a comment
jeffreylovitz
left a comment
There was a problem hiding this comment.
@renewooller Nice fix! I'm requesting one small change in comments, then will approve and merge.
It is a bit of a confusing discrepancy that user-provided field types describe 0..n columns, while properties are only represented in 2..n columns for edges and either 0..n or 1..n columns for nodes (depending on whether the node identifier is a property or not). In a later version, we might want to switch the field_types JSON format to use key-value pairs of column names and formats, rather than an array.
jeffreylovitz
left a comment
jeffreylovitz
left a comment
There was a problem hiding this comment.
Approving, will append minor change separately. Thanks, @renewooller!
2 participants
the index for field types didn't consider the offset, so caused an error when offset was being used