-
Notifications
You must be signed in to change notification settings - Fork 33
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
Datetimes have a UTC timezone by default #601
Conversation
return mockobj._index[tablename][index][field] | ||
else: # return a full row | ||
return mockobj._index[tablename][index] | ||
if index is None: # return all rows |
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 are some improvements to the testing infrastructure to make certain kinds of assertions easier to write. It doesn't require a deep review because it isn't user-facing code and it gets well-tested by virtue of being a test suite.
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.
🔬 🕵🏾♂️
@@ -585,7 +585,11 @@ def executable_blocks(self): | |||
return {**self.field_funcs(), "fake": self.fake} | |||
|
|||
def fake(self, name): | |||
return str(self.runtime_context.faker_template_library._get_fake_data(name)) | |||
val = self.runtime_context.faker_template_library._get_fake_data(name) | |||
if isinstance(val, Scalar.__args__): |
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.
Can use typing.get_args()
here instead:
if isinstance(val, Scalar.__args__): | |
if isinstance(val, T.get_args(scalar)): |
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 didn't do it exactly that way but: b622e7c
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.
@prescod Did you mean to replace this with ScalarTypes
?
return mockobj._index[tablename][index][field] | ||
else: # return a full row | ||
return mockobj._index[tablename][index] | ||
if index is None: # return all rows |
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.
🔬 🕵🏾♂️
tests/test_faker.py
Outdated
assert "+0" in dt or dt.endswith("Z"), dt | ||
assert dateparser.parse(dt).tzinfo |
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.
What about using dateparser.isoparse()
instead? parse
is pretty liberal about what it accepts.
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.
Co-authored-by: James Estevez <[email protected]>
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.
You've got Scalar.__args__
here (if you meant to change it), but LGTM otherwise.
Overrides Faker's DateTime-generating methods to return DateTimes with timezone set to UTC by default. Other timezones can be specified as a
relativedelta
. Timezone handling can be turned off by specifyingFalse
.