-
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-49 Reduce field_to_sqlalchemy_type complexity #224
RM-49 Reduce field_to_sqlalchemy_type complexity #224
Conversation
…qlalchemy.py-127-1-C901-field_to_sqlalchemy_type-is-too-complex-16
@Brunope , I pulled out the typecasting from the function map. I think I do have a preference for keeping the one/two-liners as lambdas rather than having a ton ton of fully-scoped functions in the code. Seems like it'd get crowded. |
'datetime': lambda field, driver: driver.type_for_date_plus_time(has_tz=False), | ||
'datetimetz': lambda field, driver: driver.type_for_date_plus_time(has_tz=True), | ||
'time': lambda field, driver: sqlalchemy.sql.sqltypes.TIME(timezone=False) | ||
if driver.supports_time_type() else sqlalchemy.sql.sqltypes.VARCHAR(8), |
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.
these multi-line inline if statements are kinda weirdin me out, sure it doesn't need a backslash or somethnig?
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.
I thought so too but flake8 says this is where they go, I got a hanging indent error for trying to make them look reasonable.
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.
I can think about just putting everything in parens
Generally, all the Anyway, just musing on the general state of this project, nothing for us to do about this right now |
The reason we're using type hints here in particular is that lambdas don't allow type hinting and casting works as a workaround so that we pass mypy typechecking but, yes, you're right, casting != type hinting so we wouldn't get the compile time error that we're looking for. I guess it's a good reason to use fully scoped functions? |
No description provided.