-
Notifications
You must be signed in to change notification settings - Fork 4
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
RM-48 factor to_pandas_dtype into dictionary #233
RM-48 factor to_pandas_dtype into dictionary #233
Conversation
RM-48 check if is callable RM-48 call only when complicated RM-48 factor to_pandas_dtype into dictionary
0441a25
to
f4b0137
Compare
…nit__.py-202-5-C901-RecordsSchemaField.to_pandas_dtype-is-too-complex-22
} | ||
|
||
pd_dtype = field_type_to_pd_dtype_map.get(self.field_type) | ||
if not pd_dtype: |
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.
if pd_dtype is None
raise NotImplementedError("Teach me how to handle records schema " | ||
f"type {self.field_type}") | ||
if self.field_type in ('integer', 'decimal') and callable(pd_dtype): |
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.
kind of weird to make the map but then have to special case certain types again anyway.
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.
Yeah... the other option was to have everything be a lambda again which was kind of a drag. Open to talking through ideas here on Monday!
yeah its broken on main |
…to RM-48-records_mover-records-schema-field-__init__.py-202-5-C901-RecordsSchemaField.to_pandas_dtype-is-too-complex-22
No description provided.