sugarlabs / chart

A graphing sugar activity
GNU General Public License v3.0
1 stars 7 forks source link

Bug: Messy with handling fractions #7

Closed sanjaymaniam closed 4 years ago

sanjaymaniam commented 4 years ago

If you enter a fraction, say 3/4, Charts accepts it as a 3. It always takes the first number.

Kids usually learn fractions before decimal numbers, it might make sense to support it. Even otherwise, we could prompt "fractions not supported".

Thanks.

sanjaymaniam commented 4 years ago

Is this okay? I made the following change in activity.py

def _extract_value(value):
    a, b = value.split('/')
    value = float(a)/float(b)

    decimals_found = re.findall("\d+\.\d+", str(value))
    integers_found = re.findall("\d+", str(value))

    if decimals_found != []:
        return decimals_found[0]
    elif integers_found != []:
        return integers_found[0]
    return None
chimosky commented 4 years ago

Hi @sanjaynumber99 feel free to make the changes, test and open a PR. See our contributing guide.

sanjaymaniam commented 4 years ago

Hi @chimosky, I've raised a PR. Can you tell me what I can do next?

quozl commented 4 years ago

Thanks for the idea.

You're proposing that 3/4 is converted to 0.75 rather than being displayed as 3/4. While good, this is not perfect. Perfect would be to display as 3/4 in both the input spreadsheet and the chart.

By supporting one form of expression we may expect other expressions to be attempted, such as addition, subtraction, and multiplication. Calculate activity accepts a range of expressions.

Data input right now does a few other surprising things;

I'm fine with what you suggest, @sanjay237, as it is an improvement on what we have now.

sanjaymaniam commented 4 years ago

@quozl

You're proposing that 3/4 is converted to 0.75 rather than being displayed as 3/4. While good, this is not perfect. Perfect would be to display as 3/4 in both the input spreadsheet and the chart.

My proposal was only for the computation. I agree that displaying the fraction itself avoids confusion for the younger kid, but, a good chart is standardized. I mean to say that all representations should be in the same format. So 3/4 should be written as 0.75. What do you think?

By supporting one form of expression we may expect other expressions to be attempted, such as addition, subtraction, and multiplication. Calculate activity accepts a range of expressions.

Would you like for me to check if input is a string, and if it is, evaluate it using the built-in eval() function? I feel would be a better interface then.

quozl commented 4 years ago

My proposal was only for the computation. I agree that displaying the fraction itself avoids confusion for the younger kid, but, a good chart is standardized. I mean to say that all representations should be in the same format. So 3/4 should be written as 0.75. What do you think?

Okay.

Would you like for me to check if input is a string, and if it is, evaluate it using the built-in eval() function? I feel would be a better interface then.

Yes, thanks. @chimosky, you've spoken on this issue, what do you think?

sanjaymaniam commented 4 years ago

@quozl When I try to run Chart from Sugar's fork, I run into the following error:

/usr/lib/python2.7/dist-packages/jarabe/main.py:381: Warning: g_value_transform: assertion 'G_IS_VALUE (src_value)' failed
  Gtk.main()
/usr/lib/python2.7/dist-packages/jarabe/main.py:381: Warning: unable to set property 'buddy' of type 'PyObject' from value of type '(null)'
  Gtk.main()

Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/dbus/connection.py", line 604, in msg_reply_handler
    reply_handler(*message.get_args_list(**get_args_opts))
  File "/usr/lib/python2.7/dist-packages/sugar3/activity/activityfactory.py", line 255, in _activate_reply_handler
    self._create_activity()
  File "/usr/lib/python2.7/dist-packages/sugar3/activity/activityfactory.py", line 237, in _create_activity
    stderr=log_file.fileno())
  File "/usr/lib/python2.7/subprocess.py", line 394, in __init__
    errread, errwrite)
  File "/usr/lib/python2.7/subprocess.py", line 1047, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory

However, I don't get any error when I'm running from Abus' repository. I googled a little bit and found this to be relevant, but where do I put the shell=True argument? Is this a bug?

chimosky commented 4 years ago

@quozl I agree as his change seems a better alternative that what we currently have.

quozl commented 4 years ago

@quozl When I try to run Chart from Sugar's fork, I run into the following error:

That's shell.log from the jarabe module running in Python 2. We switched to Python 3 in Sugar 0.116. Please upgrade Sugar and the Toolkit to 0.116 or later.

Or, if you can't upgrade, base your work on the python2 branch of this repository, making the pull request to the python2 branch.

@quozl I agree as his change seems a better alternative that what we currently have.

Thanks.

sanjaymaniam commented 4 years ago

@quozl I've raised a PR and referenced it to this thread. Sorry for taking too long!