Minor issues in spyder2.2rc and quick fixes

[Sylvain Corlay]

Hi everyone, 

Here are a couple of issues in the release candidate. 

1) When using "run selection"

Create a new .py file with the code

# No indentation between 'print i' and 'print i+1'

for i in range(10):

    print i

    print i+1

Put the focus on a regular python console (not ipython), select all the text in the editor and use the  "run selection or current block" button. 

This yields an indentation error. This can probably be considered as a bug. 

Fix: In base.py (widgets/sourcecode) line 416

        # If there is a common indent to all lines, remove it

        min_indent = 999

        current_indent = 0

        lines = text.split(ls)

        for i in xrange(len(lines)):

            line = lines[i]

            if line.strip():

                current_indent = _indent(line)

                min_indent = min(current_indent, min_indent)

            else:

                lines[i] = ' ' * current_indent

        text = ls.join([line[min_indent:] for line in lines])

It solves the problem for me. The number of space characters added to empty lines has to be the current indentation, for the case of nested loops. 

2) Editor widget and debug toolbar

The debug toolbar items are missing when separating the editor dock from the main window. 

Fix: In editor.py (plugins) line 903 

        self.dock_toolbar_actions = file_toolbar_actions + [None] + \

                                    source_toolbar_actions + [None] + \

                                    run_toolbar_actions + [None] + \

                                    debug_toolbar_actions +  [None] + \

                                    edit_toolbar_actions

3) Regarding issue 852

With the first point corrected as mentioned above, the "hack" of mixins.py documented already does the job to add the Matlab-like cell feature. So I don't really see why one should consider it as a hack, or why it would be less maintainable than the current version. 

-            text = unicode(cursor0.selectedText()) 

-            return len(text.strip()) == 0 or text.lstrip()[0] == '#' 

+            text = unicode(cursor0.selectedText()).lstrip() 

+            return text[:2] == '# %%' or text[:12] == '# <codecell>'

The thing is that with this modification, the behavior will already be the one you want eventually, and this Matlab-like cell feature is the most popular feature request in the tracker. To be rigorous, "block" should be replaced by "cell" in doc strings, functions names, and menus. 

I know that Carlos wants to add some graphical visualization of the cells in the editor (horizontal lines and coloring), but with this modification, the behavior of Spyder 2.2 would already be more consistent with what you guys want to do in 2.3. 

4) No warning in the case of presence of lock files

If spyder crashes and spyder.lock directory remains, spyder won't start again, and no error message is displayed. It just fails to start. Maybe a their should be at least an error message in the console or even better a pop-up window. 

Finally, I would really like to thank Carlos, Pierre and Jed for their incredible work on this IDE. I think that spyder 2.2 will really become a reference. You guys are going to become as famous as the creators of ipython, or numpy. Spyder was the missing link in the scientific python world and you created it. 

Cheers,

Sylvain

[Carlos Córdoba]

Hi Sylvain,

Thanks for your patches and suggestions. My detailed answers are below:

El 10/04/13 17:35, Sylvain Corlay escribió:

Hi everyone, 

Here are a couple of issues in the release candidate. 

1) When using "run selection"

Create a new .py file with the code

# No indentation between 'print i' and 'print i+1'

for i in range(10):

    print i

    print i+1

Put the focus on a regular python console (not ipython), select all the text in the editor and use the  "run selection or current block" button. 

This yields an indentation error. This can probably be considered as a bug. 

Fix: In base.py (widgets/sourcecode) line 416

        # If there is a common indent to all lines, remove it

        min_indent = 999

        current_indent = 0

        lines = text.split(ls)

        for i in xrange(len(lines)):

            line = lines[i]

            if line.strip():

                current_indent = _indent(line)

                min_indent = min(current_indent, min_indent)

            else:

                lines[i] = ' ' * current_indent

        text = ls.join([line[min_indent:] for line in lines])

It solves the problem for me. The number of space characters added to empty lines has to be the current indentation, for the case of nested loops. 

Yeah, I was thinking something like this would solve the problem. Thanks a lot for the patch, I'll review it later this week.

2) Editor widget and debug toolbar

The debug toolbar items are missing when separating the editor dock from the main window. 

Fix: In editor.py (plugins) line 903 

        self.dock_toolbar_actions = file_toolbar_actions + [None] + \

                                    source_toolbar_actions + [None] + \

                                    run_toolbar_actions + [None] + \

                                    debug_toolbar_actions +  [None] + \

                                    edit_toolbar_actions

Again, thanks for the patch. This seems a harmless addition, so I'll add it this weekend too.

3) Regarding issue 852

With the first point corrected as mentioned above, the "hack" of mixins.py documented already does the job to add the Matlab-like cell feature. So I don't really see why one should consider it as a hack, or why it would be less maintainable than the current version. 

-            text = unicode(cursor0.selectedText()) 

-            return len(text.strip()) == 0 or text.lstrip()[0] == '#' 

+            text = unicode(cursor0.selectedText()).lstrip() 

+            return text[:2] == '# %%' or text[:12] == '# <codecell>'

The thing is that with this modification, the behavior will already be the one you want eventually, and this Matlab-like cell feature is the most popular feature request in the tracker. To be rigorous, "block" should be replaced by "cell" in doc strings, functions names, and menus.

I disagree with you on this point. I think we need to maintain "Run selection" as a feature and add "Run cell" as a new one. This is my reasoning: I want Spyder remains as beginner friendly as possible, so if you want to select some text and send it to the console, that should still be possible too. Also, if you want to run consecutive chunks of code separated by blank lines (as it's the case now), that should be possible too. I'm not so sure about block separation using '#'. I think once cells are in we are going to eliminate it, as you're suggesting with our patch.

I know that Carlos wants to add some graphical visualization of the cells in the editor (horizontal lines and coloring), but with this modification, the behavior of Spyder 2.2 would already be more consistent with what you guys want to do in 2.3.

It's not only that I want to add visual clues to cells. There are several things to consider:

1. Since "Run cell" is a new feature, it'll need a new icon in the Run toolbar and of course a new action in the menus. We would need to add some docs about it too.

2. Cells should not be allowed inside indented code (e.g for/while loops). Someone mentioned that Matlab follows this principle and I think it's a good one, i.e. cells must enclose full code chunks.

3. We need to decide what happens when one adds only one cell separator. In that case I would like that Spyder ran all lines from the file's beginning to the separator if the cursor is before it, or from the separator to the end if the cursor is after it (I don't know how Matlab handles this case).

These points are not covered by your patch, so we (the devs) would need to put more time on it (which don't have right now). Look, I don't want to be stubborn here. If you make my points work and the feature is well tested, I'll have no problem merging your work during the 2.2 cycle (let's say in 2.2.1 or .2). The visual cell clues and all that could be added in 2.3.

4) No warning in the case of presence of lock files

If spyder crashes and spyder.lock directory remains, spyder won't start again, and no error message is displayed. It just fails to start. Maybe a their should be at least an error message in the console or even better a pop-up window.

This is very serious. Under what circumstances is this happening to you? I was very careful at selecting a lock mechanism that was able to launch Spyder again if there is a crash. In Issue 1325 we discovered it won't work on Windows if you don't have pywin32 installed. But on Linux and Mac it should work as expected.

Could you give us a reproducible test case?

Finally, I would really like to thank Carlos, Pierre and Jed for their incredible work on this IDE. I think that spyder 2.2 will really become a reference. You guys are going to become as famous as the creators of ipython, or numpy. Spyder was the missing link in the scientific python world and you created it. 

Thanks a lot for your kind words. We think that too: that something like Spyder was missing to complete the Python scientific stack.

Cheers,
Carlos

Cheers,

Sylvain

--
You received this message because you are subscribed to the Google Groups "spyder" group.
To unsubscribe from this group and stop receiving emails from it, send an email to spyderlib+...@googlegroups.com.
To post to this group, send email to spyd...@googlegroups.com.
Visit this group at http://groups.google.com/group/spyderlib?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

[Sylvain Corlay]

Hi Carlos, 

Great for points 1 and 2. Sorry for the third one (Matlab cell feature). It seems indeed to involve more coding than I expected. 

For the last issue with the spyder.block folder, I only observe this behavior with windows XP. It works nicely with Ubuntu 12.04. A way to reproduce it with windows is to kill the main python process from the task manager and to restart spyder. pywin32 is installed (PythonXY 2.7.3). I run spyder 2.2rc from the bootstrap script. However, a reason for this issue could be that spyder's folder is on a network drive. I will make some tests tomorrow. 

Best, 

Sylvain

[Sylvain Corlay]

Hi Carlos, 

I fall into the case 

                if e.args[0] == ERROR_ACCESS_DENIED:

                    return

in lockfile.py, line 44. 

It seems to be related to http://stackoverflow.com/questions/2384022/winsdk-determining-whether-an-arbitrary-pid-identifies-a-running-process-on-win

Sylvain

[Sylvain Corlay]

It seems there is a nice solution here: 

http://www.madebuild.org/blog/?p=30

Sylvain

You received this message because you are subscribed to a topic in the Google Groups "spyder" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/spyderlib/brqE6p5bpkM/unsubscribe?hl=en.
To unsubscribe from this group and all its topics, send an email to spyderlib+...@googlegroups.com.

[Carlos Córdoba]

Ok, thanks a lot for tracking down the problem and providing a solution. I'll try to integrate it with lockfile.py and post a new lockfile so you can test if things work correctly with it.

By the way, thanks for understanding the issue about cells: it would require a good deal of effort to make it work right and so we are unfortunately leaving it for the next release.

Cheers,
Carlos

El 12/04/13 15:14, Sylvain Corlay escribió:

[Sylvain Corlay]

No pb, I can test your changes when you have it.

Good luck with the last lap before the final release. 

Best, 

Sylvain

[Jed Ludlow]

On Sunday, April 14, 2013 8:32:18 PM UTC-6, Sylvain Corlay wrote:

No pb, I can test your changes when you have it.

Good luck with the last lap before the final release. 

Best, 

Sylvain

Not trying to steal Carlos's thunder, but I had a little bit of time over the weekend to look at the the "run selection" behavior and the debug toolbar issues. I just pushed a couple of changes that should solve them. If you find any additional issues with the changes, please open up an issue on the tracker.

Jed 

[Carlos Córdoba]

Thanks for pushing Sylvain changes Jed. I got a terrible flu this weekend so I couldn't do any Spyder related job =)

Sylvain, I'm attaching a new lockfile.py incorporating the changes of the blog post you referenced. Could you try it and tell us if it's working for you? I tested it in my local machines and it's doing its job as before.

Cheers,
Carlos

El 15/04/13 12:45, Jed Ludlow escribió:

[Sylvain Corlay]

Hi Carlos, 

Did not work out of the box, but there is a 6 characters fix: 

Replace

    handle = kernel32.OpenProcess(0x0400, 0, pid)

by

    handle = kernel32.OpenProcess(1, 0, pid)

   0x0400 corresponds to PROCESS_QUERY_INFORMATION
   http://msdn.microsoft.com/en-us/library/windows/desktop/ms684880(v=vs.85).aspx

By adding the line

    err = ctypes.windll.kernel32.GetLastError()

just after the call of GetExitCodeProcess.

I could see that I was getting ERROR_ACCESS_DENIED (error 5). 

Instead of manually using 0x0400, it could be cleaner to import win32con.PROCESS_QUERY_INFORMATION: 

>>> import win32con

>>> win32con.PROCESS_QUERY_INFORMATION

1024

>>> 0x0400

1024

Thanks!

Sylvain

[Sylvain Corlay]

On Tuesday, April 16, 2013 10:46:35 AM UTC-4, Sylvain Corlay wrote:

Replace

    handle = kernel32.OpenProcess(0x0400, 0, pid)

by

    handle = kernel32.OpenProcess(1, 0, pid)

I meant the contrary of course. 

S. 

[Carlos Córdoba]

Yep, I noticed :) Thanks a lot Sylvain for helping us to track down and fix this delicate issue.

I'll push these changes right away.

Carlos.

El 16/04/13 09:51, Sylvain Corlay escribió:

[Sylvain Corlay]

By the way, this solves 1344.

[Carlos Córdoba]

I don't think so. 1344 is a Mac issue that we need to solve differently.

El 16/04/13 13:17, Sylvain Corlay escribió: