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.

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

Typeconversion from float to str #14

Closed
wants to merge 1 commit into from

Conversation

Saumya-Mishra9129
Copy link
Member

@quozl @JuiP Please review.

Gtk.ListStore in Line 542 is initailized with values str.
while giving float, following errors if click on "import-freespace" or "import-journal" button.
Traceback (most recent call last):
  File "/usr/share/sugar/activities/analyzeJournal/activity.py", line 443, in __import_freespace_cb
    self._graph_from_reader(reader)
  File "/usr/share/sugar/activities/analyzeJournal/activity.py", line 425, in _graph_from_reader
    self._add_value(None, label=row[0], value=float(row[1]))
  File "/usr/share/sugar/activities/analyzeJournal/activity.py", line 432, in _add_value
    pos = self.labels_and_values.add_value(label, value)
  File "/usr/share/sugar/activities/analyzeJournal/activity.py", line 582, in add_value
    _iter = self.model.insert(path, [label, value])
  File "/usr/lib/python3/dist-packages/gi/overrides/Gtk.py", line 1007, in insert
    return self._do_insert(position, row)
  File "/usr/lib/python3/dist-packages/gi/overrides/Gtk.py", line 988, in _do_insert
    row, columns = self._convert_row(row)
  File "/usr/lib/python3/dist-packages/gi/overrides/Gtk.py", line 900, in _convert_row
    result.append(self._convert_value(cur_col, value))
  File "/usr/lib/python3/dist-packages/gi/overrides/Gtk.py", line 914, in _convert_value
    return GObject.Value(self.get_column_type(column), value)
  File "/usr/lib/python3/dist-packages/gi/overrides/GObject.py", line 210, in __init__
    self.set_value(py_value)
  File "/usr/lib/python3/dist-packages/gi/overrides/GObject.py", line 249, in set_value
    raise TypeError("Expected string but got %s%s" %
TypeError: Expected string but got 10590.3046875<class 'float'>
Traceback (most recent call last):
  File "/usr/share/sugar/activities/analyzeJournal/activity.py", line 447, in __import_journal_cb
    self._graph_from_reader(reader)
  File "/usr/share/sugar/activities/analyzeJournal/activity.py", line 425, in _graph_from_reader
    self._add_value(None, label=row[0], value=float(row[1]))
  File "/usr/share/sugar/activities/analyzeJournal/activity.py", line 432, in _add_value
    pos = self.labels_and_values.add_value(label, value)
  File "/usr/share/sugar/activities/analyzeJournal/activity.py", line 582, in add_value
    _iter = self.model.insert(path, [label, value])
  File "/usr/lib/python3/dist-packages/gi/overrides/Gtk.py", line 1007, in insert
    return self._do_insert(position, row)
  File "/usr/lib/python3/dist-packages/gi/overrides/Gtk.py", line 988, in _do_insert
    row, columns = self._convert_row(row)
  File "/usr/lib/python3/dist-packages/gi/overrides/Gtk.py", line 900, in _convert_row
    result.append(self._convert_value(cur_col, value))
  File "/usr/lib/python3/dist-packages/gi/overrides/Gtk.py", line 914, in _convert_value
    return GObject.Value(self.get_column_type(column), value)
  File "/usr/lib/python3/dist-packages/gi/overrides/GObject.py", line 210, in __init__
    self.set_value(py_value)
  File "/usr/lib/python3/dist-packages/gi/overrides/GObject.py", line 249, in set_value
    raise TypeError("Expected string but got %s%s" %
TypeError: Expected string but got 25.0<class 'float'>
@@ -579,7 +579,7 @@ def add_value(self, label, value):
elif selected:
path = int(str(self.model.get_path(selected))) + 1
try:
_iter = self.model.insert(path, [label, value])
_iter = self.model.insert(path, [label, str(value)])
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure it should be changed to str here, instead of TypeError Exception handling? Also I suggest you mention just the error and some part of the traceback in the commit message than copy pasting the entire traceback :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah actually in https://github.com/sugarlabs/AnalyzeJournal/pull/14/files#diff-03091df2633029b2c2893b03883f5296R542 , it is initialised with str. We can try Exception handling if it is not reproducible by your test system. Can you tell about your test system( earlier one also in case it is changed) ?

Choose a reason for hiding this comment

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

I agree with @JuiP. The exception handling does take care of the typecast that has been introduced in the try block

@JuiP
Copy link
Member

JuiP commented Jun 24, 2020

@Saumya-Mishra9129 Could you please explain how do I reproduce the error?

@Saumya-Mishra9129
Copy link
Member Author

@Saumya-Mishra9129 Could you please explain how do I reproduce the error?

Error is reproducible just by clicking on import-freespace and import-journal buttons. I tested with Ubuntu 20.04. I am worried how you didnt get it while porting earlier.

@JuiP
Copy link
Member

JuiP commented Jun 24, 2020

Oh, I tested again, I still don't get the error I'm using a debian 10 buster VM to test

@Saumya-Mishra9129
Copy link
Member Author

Oh, I tested again, I still don't get the error I'm using a debian 10 buster VM to test

I also tested , error is not reproduced with debian 10 buster but with Ubuntu I can get.

@JuiP
Copy link
Member

JuiP commented Jun 24, 2020

Interesting!

@Saumya-Mishra9129
Copy link
Member Author

Saumya-Mishra9129 commented Jun 25, 2020 via email

@quozl
Copy link

quozl commented Jun 29, 2020

On the face of it, converting the value from float to string is contradictory to it being converted to float by _graph_from_reader.

ChartData contains a Gtk.ListStore with data type signature (str, str), not (str, float), but the add_value method contains an exception handler for ValueError which does the convert to string,

It seems likely that a more correct solution is to recognise that TypeError is returned by Gtk.ListStore on Python 3, and ValueError on Python 2. If so, the solution will be to change the name of the exception.

It isn't clear from the commit message if this is understood; there's no description of the problem and how it is solved. There's only a couple of tracebacks.

@quozl quozl mentioned this pull request Jun 29, 2020
Merged
@Saumya-Mishra9129
Copy link
Member Author

It seems likely that a more correct solution is to recognise that TypeError is returned by Gtk.ListStore on Python 3, and ValueError on Python 2. If so, the solution will be to change the name of the exception.

I don't agree with this. Both TypeError and ValueError has different meanings in Python3.

Passing arguments of the wrong type (e.g. passing a list when an int is expected) should result in a TypeError

However a ValueError is raised when - Raised when a built-in operation or function receives an argument that has the right type but an inappropriate value

ChartData contains a Gtk.ListStore with data type signature (str, str), not (str, float), but the add_value method contains an exception handler for ValueError which does the convert to string,

_iter = self.model.insert(path, [label, value])

_iter = self.model.append([label, str(value)])

I agree but these both lines uses different methods - insert and append , they both have different meanings and signifies why a ValueError is used.

Workaround could be using another exception handler for TypeError which should convert float to str.

@JuiP
Copy link
Member

JuiP commented Jun 30, 2020

Please look at comment-#13

@quozl
Copy link

quozl commented Jun 30, 2020

@JuiP, thanks, I agree. A port to Python 3 Porting to later GObject should have changed exception handler ValueError to TypeError, and because it wasn't changed we get a TypeError reported in logs.

@Saumya-Mishra9129
Copy link
Member Author

@JuiP, thanks, I agree. A port to Python 3 Porting to later GObject should have changed exception handler ValueError to TypeError, and because it wasn't changed we get a TypeError reported in logs.

I agree. We can close this PR now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants