Skip to content
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.

Sign up for GitHub

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

Jump to bottom

#6638 fixed table display type conversion #6650

Merged
merged 1 commit into from
Jan 12, 2018
Merged

#6638 fixed table display type conversion #6650

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions beakerx/beakerx/tabledisplay/tabledisplay.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,9 @@ def convert_from_pandas(self, args, types_map):

@staticmethod
def convert_value(value, value_type):
if value_type == "time":
if value == "":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious if a falsy test would be helpful here. The current code checks specifically for value being an empty string, but if the check was if value: we would catch None, an empty list, and other falsy values too.

Not sure if that's desirable, but certainly worth mentioning.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

empty list should appear as [].
none sounds right though, what does it do now?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the context of this function, it seems we return "string" for anything that doesn't match the specific checks. If we want to return "string" even when value is None, then the current code is all good. If we want to do something else when the value is None, we could do this at the at the end of the function.

Again, I'm not sure what the ideal behavior is, but I see a potentially unhandled case.

if value:
    return "string"
raise Exception("Unknown value type")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see also #6657 #6656

converted_value = value
elif value_type == "time":
converted_value = DateType(value)
elif value_type == "double":
converted_value = value.astype('str')
Expand Down Expand Up @@ -163,10 +165,10 @@ def convert_type(object_type, value):
return "int64"
if isinstance(value, int):
return "integer"
if object_type.startswith("datetime64"):
return "time"
if isinstance(value, str):
return "string"
if "datetime" in str(object_type):
return "time"
return "string"


Expand Down