Faulty outline on method organization

nerohmot commented 4 years ago

Issue Report Checklist

[X] Searched the issues page for similar reports
[ ] Read the relevant sections of the Spyder Troubleshooting Guide and followed its advice
[X] Reproduced the issue after updating with conda update spyder (or pip, if not using Anaconda)
[ ] Could not reproduce inside jupyter qtconsole (if console-related)
[X] Tried basic troubleshooting (if a bug/error)
- [X] Restarted Spyder
- [X] Reset preferences with spyder --reset
- [ ] Reinstalled the latest version of Anaconda
- [ ] Tried the other applicable steps from the Troubleshooting Guide
[ ] Completed the Problem Description, Steps to Reproduce and Version sections below

Problem Description

Say you have a class with many methods, and the methods can be 'grouped' by adding ### Marker at the beginning of a line between 2 'sections' of methods, then the outline is picking up on this, however the indentation (and the distinguishing between method and function) is not correct.

What steps reproduce the problem?

copy the following code fragment in a file, and look at what 'Outline' is making of it

# -*- coding: utf-8 -*-

class demo(object):

    def __init__(self):
        pass

    def foo(self):
        pass

### bars 1
    def bar10(self):
        pass

    def bar11(self):
        pass

    def bar12(self):
        pass

### bars 2
    def bar20(self):
        pass

    def bar21(self):
        pass

    def bar22(self):
        pass

What is the expected output? What do you see instead?

# bars 1 and # bars2 should reside one level deeper, under their demo class, and recognized as methods instead of functions now.

Paste Traceback/Error Below (if applicable)


PASTE TRACEBACK HERE

Versions

Spyder version: 4.1.1
Python version: Python 3.7.5 (default, Oct 31 2019, 15:18:51) [MSC v.1916 64 bit (AMD64)]
IPython version: IPython 7.13.0 (but probably not related)
Qt version: 5.9.6
PyQt version: PYQT_CONFIGURATION = {'sip_flags': '-t WS_WIN -t Qt_5_9_6'}
Operating System name/version: WIN10 (probably the same on OSX and Linux, but not tested by me yet)

Dependencies


# Mandatory:
atomicwrites >=1.2.0           :  1.3.0 (OK)
chardet >=2.0.0                :  3.0.4 (OK)
cloudpickle >=0.5.0            :  1.3.0 (OK)
diff_match_patch >=20181111    :  20181111 (OK)
intervaltree                   :  None (OK)
IPython >=4.0                  :  7.13.0 (OK)
jedi =0.15.2                   :  0.15.2 (OK)
nbconvert >=4.0                :  5.6.1 (OK)
numpydoc >=0.6.0               :  0.9.2 (OK)
paramiko >=2.4.0               :  2.7.1 (OK)
parso =0.5.2                   :  0.5.2 (OK)
pexpect >=4.4.0                :  4.8.0 (OK)
pickleshare >=0.4              :  0.7.5 (OK)
psutil >=5.3                   :  5.7.0 (OK)
pygments >=2.0                 :  2.6.1 (OK)
pylint >=0.25                  :  2.4.4 (OK)
pyls >=0.31.9;<0.32.0          :  0.31.9 (OK)
qdarkstyle >=2.8               :  2.8 (OK)
qtawesome >=0.5.7              :  0.7.0 (OK)
qtconsole >=4.6.0              :  4.7.1 (OK)
qtpy >=1.5.0                   :  1.9.0 (OK)
rtree >=0.8.3                  :  0.9.3 (OK)
sphinx >=0.6.6                 :  2.4.0 (OK)
spyder_kernels >=1.9.0;<1.10.0 :  1.9.0 (OK)
watchdog                       :  None (OK)
zmq >=17                       :  18.1.1 (OK)

# Optional:
cython >=0.21                  :  0.29.15 (OK)
matplotlib >=2.0.0             :  3.1.3 (OK)
numpy >=1.7                    :  1.18.1 (OK)
pandas >=0.13.1                :  1.0.2 (OK)
scipy >=0.17.0                 :  1.4.1 (OK)
sympy >=0.7.3                  :  1.5.1 (OK)

ccordoba12 commented 4 years ago

Thanks for the bug report @nerohmot!

@steff456, please take a look at this one.

bcolsen commented 4 years ago

I didn't know you could do that.

Unfortunately the ### marker "grouping" isn't the feature(not by design at least) that's the bug. If you put the markers at the correct indentation level for the class it works as expected.

If desired, this type of arbitrary grouping would need to be done in a way that doesn't rely on indentation so it doesn't break python code. We would need to use beginning and end markers (maybe # ## and # \## or # << and # >> see #8855).

If python read comments, then the example code here would be an indentation error. This same problem happens with users using code cells inside classes and functions and so far my response has been please don't do that. Which is good advice because it leads to making broken code in your code cells.

One solution is making markers ignore indentation and never create groups. As for telling were they should go in the tree, I guess markers comments could always be siblings with the next line of code.

ccordoba12 commented 4 years ago

One solution is making markers ignore indentation and never create groups

I think this is the best solution for now. Could you help us with this @bcolsen?

bcolsen commented 4 years ago

I think the best solution might be to drop it entirely. ### (or more) is also only match for the marker and that's a code style error. It's easy to implement dropping it and it could speed up the outline explorer instead of slowing it down.

One solution is making markers ignore indentation and never create groups. As for telling were they should go in the tree, I guess markers comments could always be siblings with the next line of code.

To implement this I would have to know what the level of the next tree is and match the indentation.

bcolsen commented 4 years ago

I'll have a look at the code.

raphaelquast commented 4 years ago

I've noticed something similar... this also produces a wrong grouping:

class abc():
    def f_1():

        if True:
            def sub_f_1():
                return

        def sub_f_2():
                return

ccordoba12 commented 4 years ago

@steff456, please take a look at this one because I'd like to include a fix for it in our next release.

bcolsen commented 4 years ago

I worked on this today and I have a solution. I'll have a PR soon (tomorrow if all goes well)

ccordoba12 commented 4 years ago

Great to know!! Thanks a lot @bcolsen!

But at the end we decided to allow comments of the form ### to split the outline. Are you working along those lines?

bcolsen commented 4 years ago

But at the end we decided to allow comments of the form ### to split the outline. Are you working along those lines?

I don't have a great view of how that would be possible without using the tab level of the comments.

For example, I would like them to work as the original poster wants:

class demo(object):

    def __init__(self):
        pass

    def foo(self):
        pass

    # ## bars 1
    def bar10(self):
        pass

    def bar11(self):
        pass

    def bar12(self):
        pass

    # ## bars 2
    def bar20(self):
        pass

    def bar21(self):
        pass

    def bar22(self):
        pass

Would give:

C: demo
    M: __init__
    M: foo
    #: bars 1
        M: bar10
        M: bar11
        M: bar12
    #: bars 2
        M: bar20    
        M: bar21
        M: bar22

Do we want this to do with:

# ## top level
class demo(object):

    def __init__(self):
        pass

    def foo(self):
        pass

Does it fold until the next comment?

If so, one simple way to implement that is to make half levels(I would implement as even and odd ints) have the comments on a half levels based on their tab level.

A class or function would have a tab level of 1,3,5,7,... and comments would have 0,2,4,6,... based on the indentation level of the comment. This would allow the code to have a lot of flexibility, but the indentation level would matter.

Without using indentation level there is a some ambiguity whether a comment is inside or outside a class or function.

What did you have in mind?

KardoPaska commented 4 years ago

Here is another example for you guys...

Note that between section 1 and 2 an indentation level was skipped. Stack

P.S. I love this feature! please dont drop it! Thanks!

bcolsen commented 4 years ago

@KardoPaska If you want to see how the fix is going. I could use some help in testing my https://github.com/spyder-ide/spyder/pull/12343

spyder-ide / spyder