Python3 and other fixes #11

JuiP · 2020-04-03T10:10:26Z

Tested
Errors- none

Fix noisy logs
Fix Gtk-WARNINGS
Fix Errors in _object_chooser when no object is selected
Port to python 3
Added new translations to hi.po
Refreshed the pot file, fetched new translations and deleted the empty ones

@chimosky @quozl Please test, review and merge :)

… after python 3.3

chimosky

You can demote logging instead of removing them entirely, you can demote to debug.

readers.py

chimosky · 2020-04-03T14:20:21Z

Thanks, reviewed 00e5df3.

JuiP · 2020-04-03T15:37:09Z

@chimosky changed as suggested!

quozl · 2020-04-04T02:22:42Z

@Hrishi1999, is further work on AnalyzeJournal compatible with your plan for dashboard? Or is there something you can use here?

chimosky · 2020-04-04T14:50:35Z

Tested, reviewed. Thanks.

In 1f1bb36, I think using stat.f_bsize is better than indexing to access them as it makes it easier at a glance to know which member of the statvfs structure is used.

Is there a particular reason why you used collections.Callable in 97fdd0f and not the builtin callable?

The object chooser created by the Read Turtle data button doesn't have focus until activity closes, also occurs at master. Can you take a look?, thanks.

Logs show this error.

1586012753.417484 WARNING root: No icon with the name line was found in the theme.
1586012753.426675 WARNING root: No icon with the name line was found in the theme.

Hrishi1999 · 2020-04-04T16:10:41Z

@quozl The Dashboard Activity was supposed to be an extension of AnalyzeJournal integrated with Sugar's Journal. Dashboard does use sugarpycha, but the version of sugarpycha in Dashboard is already in python3. In future, to avoid conflicts, Dashboard can use AnalyzeJournal's sugarpycha version as a submodule or vice-versa.

JuiP · 2020-04-05T17:50:48Z

Thanks for testing :)

Is there a particular reason why you used collections.Callable in 97fdd0f and not the builtin callable?

I saw chart activity used sugarpycha which was using collections.Callable instead of builtin callable when it was ported to python3 , so I just used it to maintain similarity in the code of sugarpycha

In 1f1bb36, I think using stat.f_bsize is better than indexing to access them as it makes it easier at a glance to know which member of the statvfs structure is used.

Yes, I agree with you. Except for making the code more readable it isn't much of a difference. I will make the change if changes to AnalyzeJournal are compatible. Should I go ahead with changes @quozl @chimosky ?

The object chooser created by the Read Turtle data button doesn't have focus until activity closes, also occurs at master. Can you take a look?, thanks.

Doesn't c02848e fix this problem? Could you please explain what you meant by doesn't have focus?

Logs show this error.

Sorry, I didn't consider that as an error. It was a warning I didn't know the cause of. Any suggestions what might have caused it / what could probably fix it ?

Hrishi1999 · 2020-04-05T18:47:52Z

Tested, 1f1bb36
The problem with ObjectChooser does exist. It doesn't have focus, that is, ObjectChooser appears behind the activity, you will see the ObjectChooser only when you close the activity or tab our.

quozl · 2020-04-06T00:12:58Z

@quozl The Dashboard Activity was supposed to be an extension of AnalyzeJournal integrated with Sugar's Journal. Dashboard does use sugarpycha, but the version of sugarpycha in Dashboard is already in python3. In future, to avoid conflicts, Dashboard can use AnalyzeJournal's sugarpycha version as a submodule or vice-versa.

Sorry, my question must have been a bit too vague. What I'm saying is that we seem to have two parallel developments to choose from; either a Dashboard in Sugar, or the AnalyzeJournal activity being ported to Python 3 so it can be used in Sugar again. I'm fine with competition to choose the best solution, but would it be more effective to bring the AnalyzeJournal features into the draft Dashboard feature, or to take some of the draft Dashboard features and bring them into AnalyzeJournal?

Hrishi1999 · 2020-04-06T13:03:29Z

Sorry, it seems my last answer wasn't focused on the question. I would like the features to be ported to Dashboard instead since it is going to be integrated with the Journal. But there are some features in AnalyzeJournal which may not make sense if we implement in the Dashboard.
@chimosky Do you have any suggestions?

chimosky · 2020-04-06T15:19:43Z

I agree with @Hrishi1999, some features of AnalyzeJournal should be ported to Dashboard not all features.

chimosky · 2020-04-06T16:10:05Z

Doesn't c02848e fix this problem? Could you please explain what you meant by doesn't have focus?

Tested again and the issue no longer persists, when I said "doesn't have focus" I meant when the button is clicked, the object chooser window didn't come up like it's supposed to but came up behind the activity canvas and was only visible when the activity was closed.

Yes, I agree with you. Except for making the code more readable it isn't much of a difference. I will make the change if changes to AnalyzeJournal are compatible. Should I go ahead with changes @quozl @chimosky ?

Yes it doesn't make any functional difference, my earlier comment was for readability.
Yes you can go on with the change, thanks.

Sorry, I didn't consider that as an error. It was a warning I didn't know the cause of. Any suggestions what might have caused it / what could probably fix it ?

The warning is caused when _get_icon_info of sugar3.graphics.icon._IconBuffer is called with just an icon_name arg and a Gtk.IconTheme.lookup is performed and the query returns None.
The lookup is performed on an icon with the name line and there's no icon with such name in icons/, you'll have to figure out why it happens.
Hope that helps.

srevinsaju

The tables might not be evenly spaced. On launching the activity. it looks like this

These are the tracebacks I got

~~I guess @quozl mentioned some of the errors previously~~ Nvm

README.md

activity.py

readers.py

Saumya-Mishra9129

1586012753.417484 WARNING root: No icon with the name line was found in the theme.
1586012753.426675 WARNING root: No icon with the name line was found in the theme.

@JuiP I had a quick review of code and found that the code first when written thought to implement line chart of Sugarpycha also along with pie, vbar and hbar. As look at charts.py Line 121 and activity.py line 483.. But not adding line chart in activity.py causing the error as seen by @srevinsaju and above mentioned error also. Thanks :)

Saumya-Mishra9129 · 2020-05-12T11:03:14Z

Reviewed again 4d4eb12, when we click on Read Turtle Data button, Object chooser window doesnt appear in front , it is behind of canvas window thats why we are not able to see that time and when we click on stop button we can see object chooser window , it means it was hidden.
Also I am getting this error,

JuiP · 2020-05-12T11:10:04Z

Hey @Saumya-Mishra9129, can you please tell me how did you get that error? I have nothing in my logs, also do this error also occur when you test master?

srevinsaju · 2020-05-12T11:10:39Z

I tested 4d4eb12. ~~Seems like the ObjectChooser is fixed now (only for LoadTurtle file)~~. See #11 (comment)

However, this remains unfixed

srevinsaju · 2020-05-12T11:14:09Z

Interesting results. Seems like the bug for Objectchooser is unfixed, hinting it might be a bug in the call of ObjectChooser in the code. (Please verify the signature of the functions have been / not been changed over)

Simple reproducer:

Restart Sugar
Clone and install ColorDeducto
Run colordeducto, test the Object Chooser
Close colordeduct
Open AnalyzeJournal
Test ObjectChooser, It works fine

Restart sugar
open analyzeJournal
Try clicking the Turtle Load option
You will notice no window is seen

JuiP · 2020-05-12T11:15:40Z

Seems like the ObjectChooser is fixed now

Well that's weird, it isn't fixed for me and Saumya. Before 4d4eb12, it was fixed for me.

@srevinsaju please tell me your test method for getting the error.

srevinsaju · 2020-05-12T11:16:47Z

@JuiP

Well that's weird, it isn't fixed for me and Saumya. Before 4d4eb12, it was fixed for me.

See #11 (comment)

srevinsaju · 2020-05-12T11:20:02Z

To get the ValueError on #11 (comment), click this

To get the IndexError, you have to click Save Image option

Hey <> , can you please tell me how did you get that error? I have nothing in my logs, also do this error also occur when you test master?

@JuiP, is your logging system alright?

Please configure sugar in DEBUG mode to make sure you get all the logs if you are not running within terminal.
Make sure all the methods / functions are completely tested before next review
To test the code, please do a coverage test. If you are getting a coverage percentage below 85%, you might need to expect more bugs in your code.

See the coverage guide in sugar-docs

JuiP · 2020-05-12T12:17:58Z

I think the coverage is low because I can't test the Turtle file as the object chooser appears behind the window. I have tried clicking all sorts of buttons, and saving each graph as image.

Please configure sugar in DEBUG mode to make sure you get all the logs if you are not running within terminal.

I am running from within the sugar-desktop terminal with command sugar-activity3, I tried doing what you displayed in the images, tried read Freespace Data and save as image. I did not get the tracebacks. The image is saved as png in my journal. I don't understand why is there a difference when testing. I am using a debian build (If that helps)

srevinsaju · 2020-05-12T12:49:20Z

@JuiP I am unable to save a png of the graph for some reason.
Did you try this to enable logging?

I might have been suspicious if the error I had received might be because of something wrong with my system; but as @Saumya-Mishra9129 were also able to reproduce it (1) here, so I was quite suspicious if something had gone wrong during the Port

Regarding #11 (comment), I can confirm that this is a bug which possibly happened due to python3 port (I have removed all of python2 from my system, so can't test master).

An immediate correction on

https://github.com/JuiP/AnalyzeJournal/blob/python3/activity.py#L573-L576

would be to add another Exeception catch

change this

        try:
            _iter = self.model.insert(path, [label, value])
        except ValueError:
            _iter = self.model.append([label, str(value)])

to

        try:
            _iter = self.model.insert(path, [label, value])
        except (ValueError, TypeError):
            _iter = self.model.append([label, str(value)])

In the long run, you might need to find the fix for why it actually comes out as int or float instead of str; But the above patch would fix the error, in a legal way.

I am not sure, however of this error and the ObjectChooser Error by simply looking at code, (might need to debug and compare other upstream activities to see what makes them work, and not this)

Hrishi1999 · 2020-05-12T13:06:25Z

Tested 4d4eb12. I couldn't seem to reproduce the tracebacks provided by @Saumya-Mishra9129 and @srevinsaju. One weird thing to notice is that ObjectChooser only worked once for me, after rebooting the system, it was only visible when the activity is closed.

Saumya-Mishra9129 · 2020-05-12T13:08:10Z

I tested 4d4eb12. ~~Seems like the ObjectChooser is fixed now (only for LoadTurtle file)~~. See #11 (comment)

However, this remains unfixed

I am not getting this error as mentioned by @srevinsaju , but I am getting index error.

Saumya-Mishra9129 · 2020-05-12T13:19:34Z

@JuiP Add line chart here as well . I think It can help for index error.

srevinsaju · 2020-05-12T13:20:43Z

@Saumya-Mishra9129 I were talking about IndexError

I have two errors:

TypeError (only by me)
IndexError (faced by you and me)

JuiP · 2020-05-12T15:53:36Z

@JuiP Add line chart here as well . I think It can help for index error.
Well yes this seems to be the most likely cause for the error, can you test the change:

self.chart_type_buttons = [add_vbar_chart,
                                   add_hbar_chart,
                                   add_line_chart,
                                   add_pie_chart]

@Saumya-Mishra9129 You will have to test this patch for me before I add it to the commit because I am unable to reproduce the traceback. I'm not sure why, but logically if this fixes it then I should have got the traceback too 😕

An immediate correction on would be to add another Exeception catch

Yes, that could be the most intuitive way to fix it but I'll wait till I find out the cause, because none of us seem to get this error in the logs. @srevinsaju

Saumya-Mishra9129 · 2020-05-12T18:07:55Z

self.chart_type_buttons = [add_vbar_chart,
add_hbar_chart,
add_line_chart,
add_pie_chart]

@JuiP I tested it, working fine not getting index error anymore.

srevinsaju · 2020-05-12T18:39:24Z

rm -r ~/.sugar

fixed my problem 🤣

chimosky · 2020-05-12T23:34:29Z

On testing again, this issue is still there for me. If someone has tested activities which use ObjectChooser( I think deducto uses it), can you confirm if the same issue is seen in those activities too? If it is then the fix should be made in sugar-toolkit-gtk3 @chimosky @quozl

The problem isn't from the toolkit, you should specify the parent kwarg as self when ObjectChooser is called, also add an else statement to the if statement in self._object_chooserthat binds file_path to None.

Please configure sugar in DEBUG mode to make sure you get all the logs if you are not running within terminal.

@srevinsaju You don't need to have sugar in DEBUG mode to see logs.

am running from within the sugar-desktop terminal with command sugar-activity3, I tried doing what you displayed in the images, tried read Freespace Data and save as image. I did not get the tracebacks. The image is saved as png in my journal. I don't understand why is there a difference when testing. I am using a debian build (If that helps)

You should run the activity from the activity ring as that's how a user would run it and not from the terminal.

@Saumya-Mishra9129

- Fixes index error : reported by @Saumya-Mishra9129 and @srevinsaju - Fixed : Object Chooser focus

JuiP · 2020-05-13T08:35:19Z

The problem isn't from the toolkit, you should specify the parent kwarg as self when ObjectChooser is called

Thanks for the hint @chimosky ! Fixed it in ef16be6

also add an else statement to the if statement in self._object_chooserthat binds file_path to None.

I have actually taken care of it by exception handling in c02848e. It does the same thing :)

You should run the activity from the activity ring as that's how a user would run it and not from the terminal.

Sorry, I didn't make myself clear. I do run the activity from the ring and from the terminal (to test coverage or test just minor code changes)

I tested this activity, works perfectly for me @srevinsaju @Saumya-Mishra9129 @Hrishi1999 please review and test. Thanks :)

If it's approved by all then maybe this PR is good to merge? :) @chimosky @quozl ?

activity.py

- Removed chooser = None assignment - Removed what_filter kwarg

.flake8

quozl

Reviewed.

README.md

activity.py

charthelp.py

icons/line.svg

readers.py

- fixed unnecessary capitalization and typos in README.md - Use _logger instead of calling the module - Change back the wording in charthelp.py - Used stat.f_blocks and f_bavail ... instead of indexing method

JuiP · 2020-05-21T03:14:28Z

Is this good to merge? Anymore suggested changes? @quozl @chimosky

JuiP and others added 13 commits April 3, 2020 10:32

Fix noisy logs

4b46f3a

Fix Gtk-WARNING: Can't set a target list till gtk_drag_set() is called

8a37c39

Run 2to3

b501048

update exec and replace statvfs with disk_usage as statvfs is removed…

00e5df3

… after python 3.3

sugarpycha: change callable to isinstance and more changes

97fdd0f

flake8 fix

6dcec36

Fix traceback when no object is selected in _object_chooser

c02848e

Fetch translations and delete empty translations

aa9065c

refresh pot file

3699390

Add .flake8, not added in flake8 fix

12d739d

Added Screenshots

fcd40b6

Rename screenshots

a428d4e

Update README.md

cfa54d5

chimosky requested changes Apr 3, 2020

View reviewed changes

readers.py Outdated Show resolved Hide resolved

JuiP added 2 commits April 3, 2020 20:51

demote logging

c813d97

Update exec and use os.statvfs instead of shutil.disk_usage

1f1bb36

srevinsaju suggested changes Apr 7, 2020

View reviewed changes

README.md Show resolved Hide resolved

activity.py Outdated Show resolved Hide resolved

activity.py Outdated Show resolved Hide resolved

readers.py Outdated Show resolved Hide resolved

Saumya-Mishra9129 reviewed Apr 12, 2020

View reviewed changes

JuiP added 2 commits May 13, 2020 13:00

Added improvements:

ef16be6

- Fixes index error : reported by @Saumya-Mishra9129 and @srevinsaju - Fixed : Object Chooser focus

Flake8 fix for E501: line too long

8181469

chimosky requested changes May 13, 2020

View reviewed changes

activity.py Outdated Show resolved Hide resolved

Changes to ObjectChooser

a7f1b91

- Removed chooser = None assignment - Removed what_filter kwarg

quozl reviewed May 18, 2020

View reviewed changes

.flake8 Outdated Show resolved Hide resolved

quozl reviewed May 18, 2020

View reviewed changes

JuiP added 2 commits May 18, 2020 11:42

More Changes

14f0084

- fixed unnecessary capitalization and typos in README.md - Use _logger instead of calling the module - Change back the wording in charthelp.py - Used stat.f_blocks and f_bavail ... instead of indexing method

Added end of line marker

137edbd

JuiP mentioned this pull request May 20, 2020

End of line sugarlabs/chart#10

Merged

quozl merged commit 2574bb0 into sugarlabs:master May 21, 2020

JuiP deleted the python3 branch May 21, 2020 04:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Python3 and other fixes #11

Python3 and other fixes #11

JuiP commented Apr 3, 2020

chimosky left a comment

chimosky commented Apr 3, 2020

JuiP commented Apr 3, 2020

quozl commented Apr 4, 2020

chimosky commented Apr 4, 2020 •

edited

Loading

Hrishi1999 commented Apr 4, 2020

JuiP commented Apr 5, 2020 •

edited

Loading

Hrishi1999 commented Apr 5, 2020

quozl commented Apr 6, 2020

Hrishi1999 commented Apr 6, 2020

chimosky commented Apr 6, 2020

chimosky commented Apr 6, 2020 •

edited

Loading

srevinsaju left a comment •

edited

Loading

Saumya-Mishra9129 left a comment •

edited

Loading

Saumya-Mishra9129 commented May 12, 2020 •

edited

Loading

JuiP commented May 12, 2020

srevinsaju commented May 12, 2020 •

edited

Loading

srevinsaju commented May 12, 2020

JuiP commented May 12, 2020 •

edited

Loading

srevinsaju commented May 12, 2020

srevinsaju commented May 12, 2020 •

edited

Loading

JuiP commented May 12, 2020

srevinsaju commented May 12, 2020 •

edited

Loading

Hrishi1999 commented May 12, 2020 •

edited

Loading

Saumya-Mishra9129 commented May 12, 2020

Saumya-Mishra9129 commented May 12, 2020 •

edited

Loading

srevinsaju commented May 12, 2020

JuiP commented May 12, 2020 •

edited

Loading

Saumya-Mishra9129 commented May 12, 2020

srevinsaju commented May 12, 2020

chimosky commented May 12, 2020 •

edited

Loading

JuiP commented May 13, 2020 •

edited

Loading

quozl left a comment

JuiP commented May 21, 2020

Python3 and other fixes #11

Python3 and other fixes #11

Conversation

JuiP commented Apr 3, 2020

chimosky left a comment

Choose a reason for hiding this comment

chimosky commented Apr 3, 2020

JuiP commented Apr 3, 2020

quozl commented Apr 4, 2020

chimosky commented Apr 4, 2020 • edited Loading

Hrishi1999 commented Apr 4, 2020

JuiP commented Apr 5, 2020 • edited Loading

Hrishi1999 commented Apr 5, 2020

quozl commented Apr 6, 2020

Hrishi1999 commented Apr 6, 2020

chimosky commented Apr 6, 2020

chimosky commented Apr 6, 2020 • edited Loading

srevinsaju left a comment • edited Loading

Choose a reason for hiding this comment

Saumya-Mishra9129 left a comment • edited Loading

Choose a reason for hiding this comment

Saumya-Mishra9129 commented May 12, 2020 • edited Loading

JuiP commented May 12, 2020

srevinsaju commented May 12, 2020 • edited Loading

srevinsaju commented May 12, 2020

JuiP commented May 12, 2020 • edited Loading

srevinsaju commented May 12, 2020

srevinsaju commented May 12, 2020 • edited Loading

JuiP commented May 12, 2020

srevinsaju commented May 12, 2020 • edited Loading

Hrishi1999 commented May 12, 2020 • edited Loading

Saumya-Mishra9129 commented May 12, 2020

Saumya-Mishra9129 commented May 12, 2020 • edited Loading

srevinsaju commented May 12, 2020

JuiP commented May 12, 2020 • edited Loading

Saumya-Mishra9129 commented May 12, 2020

srevinsaju commented May 12, 2020

chimosky commented May 12, 2020 • edited Loading

JuiP commented May 13, 2020 • edited Loading

quozl left a comment

Choose a reason for hiding this comment

JuiP commented May 21, 2020

chimosky commented Apr 4, 2020 •

edited

Loading

JuiP commented Apr 5, 2020 •

edited

Loading

chimosky commented Apr 6, 2020 •

edited

Loading

srevinsaju left a comment •

edited

Loading

Saumya-Mishra9129 left a comment •

edited

Loading

Saumya-Mishra9129 commented May 12, 2020 •

edited

Loading

srevinsaju commented May 12, 2020 •

edited

Loading

JuiP commented May 12, 2020 •

edited

Loading

srevinsaju commented May 12, 2020 •

edited

Loading

srevinsaju commented May 12, 2020 •

edited

Loading

Hrishi1999 commented May 12, 2020 •

edited

Loading

Saumya-Mishra9129 commented May 12, 2020 •

edited

Loading

JuiP commented May 12, 2020 •

edited

Loading

chimosky commented May 12, 2020 •

edited

Loading

JuiP commented May 13, 2020 •

edited

Loading