Removing convert button

[Aniket21mathur]

Previously the convert button has to be clicked to calculate results.

Now on pressing enter the activate signal will call the "_call" function and results will be calculated.

Fixes https://github.com/sugarlabs/convert/issues/16

[quozl]

Thanks. Tested. Comments;

• [ ] also run genpot target to update the POT file.

Waiting on @chimosky as a recent contributor to Convert.

[Aniket21mathur]

Thanks for reviewing. Updated the POT file :smile: .

[chimosky]

@quozl thanks, tested.

[quozl]

Should the conversion be on each change to the value (as described in the first comment of the issue), or when enter key is pressed (as done in this pull request)?

[Aniket21mathur]

@chimosky please suggest. Thanks!

[quozl]

Yes, please be the tie breaker, @chimosky. My opinion is in https://github.com/sugarlabs/convert/issues/16, and @Aniket21mathur's opinion is in the commits. :grin:

[chimosky]

I agree with @quozl as described in #16, It'll be great for the conversion to be automatic each time rather than wait for the enter key to be pressed. It'll be great to also have a placeholder-text instead of setting a text because the user would have to delete the text on the first click of the entry box if the digit isn't what they wanted to convert, when they can just click and enter the convert value.

[Aniket21mathur]

@chimosky I automated the conversions on every change of values and also placed a placeholder instead of the value "1". Sorry for the delay.Thanks!

[chimosky]

Tested works fine, I'd like to know why you added the statement on line 201 as it has no effect on the code.

[Aniket21mathur]

Thanks for reviewing. I added the line to check whether the input is a float number or not, but It is checked in the update_label function also, so there is no need of that line.Thanks, updated the file. :smile:

[quozl]

Thanks. Tested on Ubuntu 18.04.

A traceback happens if I choose two units and press the swap button;

Traceback (most recent call last):
  File "/usr/share/sugar/activities/Convert.activity/activity.py", line 242, in _flip
    value = float(self.label.get_text().split(' ~ ')[1])
IndexError: list index out of range

I'm puzzled by the exception handler in _call, why is it there?

[Aniket21mathur]

I'm puzzled by the exception handler in _call, why is it there?

When the activity is started the value of the text box is a string placeholder and there will also be chances when the text box is empty . Since update_label function is called on every change of value of text box, for above cases this line

new_value = locale.format(fmt, float(num_value))

will throw an error due to float(), num_value being a string. Since update_label function is called in _call function, I used an exception handler in that function.

A trace back happens if I choose two units and press the swap button;

The reason for the trace back is that initially "self.label.get_text()" is an empty value, so adding an exception handler in the flip function will fix it. Thanks!

[quozl]

The risk of an unspecified or wildcard exception handler is that it hides any other problems. Also they make code harder to read.

Could you put the exception handler around the code that may fail, and use the name of the exception?

Could you check for the condition that would throw the exception?

[Aniket21mathur]

There are 2 types of exceptions faced till now:- 1) IndexError: list index out of range, in this line of the flip function

value = float(self.label.get_text().split(' ~ ')[1])

Condition:- Start the activity, click on the flip button without entering any number in the text box. Reason:-self.label.get_text() is empty and do not contain '~'.

2)ValueError: could not convert string to float.

locale.format(fmt, float(num_value)) --(present multiple times) float(self.value_entry.get_text().replace(',', ''))

This exception is caused in these lines.The first line in the update_label function and the second in the convert function (which is called through the update_label function). Condition: Just start the activity! The textbox contains a string placeholder leading to exception. Reason: Non integer value causes the exception. For this case instead of using the exception handler for each and every line won't it be better to use it for the entire update_label function? What do you suggest? Thanks!

[chimosky]

I agree with @quozl.

For this case instead of using the exception handler for each and every line won't it be better to use it for the entire update_label function? What do you suggest? Thanks!

The exceptions raised were handled in the various callbacks you cited above. You can fix this by deleting the _call callback and removing all calls to it - as there's no use for it - and connecting the changed signal of the entry to the self._update_label callback and add the entry as an argument to the callback, also replace the call to _call in the flip callback with a call to self._update_label, passing the entry as an argument.

Also change the exception handler in _update_label to a BaseException, the exception handler you added in the flip callback has no effect too and you can remove that.

[rhl-bthr]

@Aniket21mathur ping

[chimosky]

@quozl kindly test.

[Aniket21mathur]

Thank you very much @chimosky . I tested it. Looks great.

[quozl]

Thanks. I tested.

• [ ] changing either unit does not recalculate,

I saw some other problems that were also affecting master, so I put them into a new issue.

[quozl]

Thanks. Merged as 2ac71c970f695fc1f9eb7a6820bd6fff2d87456a.

However, I did not merge 053ce2f, as although default units were set, the units were unreasonable; Cables for distance. Use SI units.