Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pressing "Clear comparison" in Profiler while its running stops it and raises a TypeError #6444

Closed
chengtian5huang opened this issue Feb 13, 2018 · 7 comments · Fixed by #20064

Comments

@chengtian5huang
Copy link

Description

when profiler is running somecode(any code) click "clear comparison".
the code stop running and spyder pump up an error as following:

File "c:\users\53282\appdata\local\programs\python\python36\lib\site-packages\spyder_profiler\widgets\profilergui.py", line 264, in
self.finished(ec, es))
File "c:\users\53282\appdata\local\programs\python\python36\lib\site-packages\spyder_profiler\widgets\profilergui.py", line 326, in finished
self.output = self.error_output + self.output
TypeError: must be str, not NoneType

i found this maybe caused by this:

    def show_data(self, justanalyzed=False):
        if not justanalyzed:
            self.output = None
        self.log_button.setEnabled(self.output is not None \
                                   and len(self.output) > 0)
        self.kill_if_running()

so i edit it:

    def show_data(self, justanalyzed=False):
        if not justanalyzed:
            self.output = ''
        self.log_button.setEnabled(self.output is not None \
                                   and len(self.output) > 0)
        #self.kill_if_running()

and now "clear comparison" wont stop the code running and no error will pump up.

Version and main components

  • Spyder Version: 3.2.6
  • Python Version: 3.6.4
  • Qt Versions: 5.10.0, PyQt5 5.10 on Windows

Dependencies

pyflakes >=0.6.0 :  1.6.0 (OK)
pycodestyle >=2.3:  2.3.1 (OK)
pygments >=2.0   :  2.2.0 (OK)
pandas >=0.13.1  :  0.22.0 (OK)
numpy >=1.7      :  1.14.0 (OK)
sphinx >=0.6.6   :  1.6.7 (OK)
rope >=0.9.4     :  0.10.7 (OK)
jedi >=0.9.0     :  0.11.1 (OK)
nbconvert >=4.0  :  5.3.1 (OK)
sympy >=0.7.3    :  None (NOK)
cython >=0.21    :  None (NOK)
qtconsole >=4.2.0:  4.3.1 (OK)
IPython >=4.0    :  6.2.1 (OK)
pylint >=0.25    :  1.8.2 (OK)

@CAM-Gerlach
Copy link
Member

Thanks for reporting; I was able to reproduce this. If it works, it works,, though perhaps there's a cleaner fix, perhaps one step up the chain in the function called by the "clear comparison" button, as this seems a bit kludgy to the untrained eye. In any case, we'd be glad to review a PR with this fix; you could probably mock a test but probably better to just run a "real world" one with qtbot, running on a test file and clicking the appropriate buttons.

@CAM-Gerlach CAM-Gerlach changed the title in Profiler "clear comparison" button stops code running! and show an error! Pressing "Clear comparison" in Profiler while its running stops it and raises a TypeError Feb 13, 2018
@CAM-Gerlach CAM-Gerlach added this to the v3.2.x milestone Feb 13, 2018
@chengtian5huang
Copy link
Author

yes, i am one with untrained eyes(at least for now) and know little about qtbot/qt. so maybe i wont be capable to fix this "in a cleaner way".

i dont really understand what u mean by "real world".
do u mean profile another piece of code and try "clear comparison" button to see if it is all ok? i have done that, and noticed some side-effects mocking it this way.

@CAM-Gerlach
Copy link
Member

i am one with untrained eyes(at least for now) and know little about qtbot/qt

No worries! You can just go ahead and submit a PR, and if there's a cleaner way I'm sure @ccordoba12 or someone will point it out during a review. I didn't know anything about qt/qtbot either going in, but the basics were easier than they seemed; you can look at the other tests to get a sense of how to do it. I could see two approaches: Either way, you'd presumably want to create two test files in the spyder/plugins/tests directory, one to test the behavior itself, with something like

import time
time.sleep(3)

and the other with arbitrary content to load as comparison data,

import time
print("Testing profiler")
time.sleep(0.1)

and create a new profiler object/window, as presumably the other tests do.

Then, you would need to do four actions:

  1. Run the profiler on the second, comparison file.
  2. Load those results
  3. Run the profiler on the first, test file and
  4. Trigger the method linked to the "Clear results" button.

These can be accomplished either by calling the relevant methods for each one directly, or with doing qtbot.mouseClick() on the relevant buttons and/or qtbot.keyClick() on the hotkeys. But again, if you can't get it working just submit the PR and we can help you further.

i dont really understand what u mean by "real world". do u mean profile another piece of code and try "clear comparison" button to see if it is all ok?

Yes, exactly, and just automating it with method calls or qtbot.

i have done that, and noticed some side-effects mocking it this way.

Oh; could you explain further?

@ccordoba12 ccordoba12 modified the milestones: v3.x, Not sorted Oct 11, 2018
@goanpeca
Copy link
Member

goanpeca commented Nov 29, 2019

@bcolsen, @jnsebgosselin what are your thoughts on how this should flow?

Screen Shot 2019-11-28 at 20 55 32

We could just disable the "clear comparison" while it is running? or should we not stop the process?

@bcolsen
Copy link
Member

bcolsen commented Nov 29, 2019

I didn't even realize that you could make comparisons. I thought that you could just save and load data to see it again later. Maybe Load Comparison Data or something. Also being able to just compare to the previous result without saving would be great.

Sorry side tracked. As for this issue I don't see why I shouldn't be able to clear a comparison, load data or save data while the profiler is running. From a usability stand point it makes sense.

If we have technical limitations for doing these things while the profiler is running then disabling buttons is the way to go. I also don't think that a user needs to clear a comparison, load data or save data while the profiler is running.

@goanpeca
Copy link
Member

Thanks for the input and the suggestion! We need to work a bit on the profiler to update with other panes. This one is a bit orphaned. :-)

@goanpeca
Copy link
Member

Also being able to just compare to the previous result without saving would be great.

I think it is already done if you keep running the profiler

@goanpeca goanpeca modified the milestones: not sorted, v5.0beta1 Nov 29, 2019
@goanpeca goanpeca removed this from the v5.0beta1 milestone Feb 18, 2020
@dalthviz dalthviz added this to the v5.4.1 milestone Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants