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.

Sign up for GitHub

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

Jump to bottom

PR: Profile in the IPython console to gain access to variables and get a result in case of cancellation #15372

Open
wants to merge 69 commits into
base: master
Choose a base branch
from
Open

PR: Profile in the IPython console to gain access to variables and get a result in case of cancellation #15372

wants to merge 69 commits into from

Conversation

impact27
Copy link
Contributor

@impact27 impact27 commented Apr 21, 2021 •
edited by ccordoba12
Loading

Description of Changes

Add profile_file and profile_cell functions to the shell with the same signature as runcell/debugcell and runfile/debugfile

This solves a few issues I have with profiling, including:

Issue(s) Resolved

Fixes #2378

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct:

@pep8speaks
Copy link

pep8speaks commented Apr 21, 2021 •
edited
Loading

Hello @impact27! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 242:1: W293 blank line contains whitespace

Line 204:80: E501 line too long (95 > 79 characters)
Line 205:80: E501 line too long (89 > 79 characters)
Line 206:80: E501 line too long (98 > 79 characters)
Line 348:80: E501 line too long (89 > 79 characters)

Comment last updated at 2024-10-04 07:23:20 UTC

@impact27 impact27 changed the base branch from master to 5.x April 21, 2021 12:07
@impact27 impact27 changed the title PR: Profile script PR: Profile in the ipython console to gain access to variables and get a result in case of cancellation Oct 8, 2021
@ccordoba12
Copy link
Member

Hey @impact27, please post an animated gif/video of the new behavior.

@impact27
Copy link
Contributor Author

impact27 commented Oct 10, 2021 •
edited
Loading

plot

List of improvements:

  • Access to local variables in the console
  • Can compare several runs by profiling in different consoles
  • Can profile in the same environment as the code is run
  • Can get a result even if the run is interrupted (For example if the code reaches an infinite loop)
  • Adds %profile magic to profile a function or short line of code
  • Can profile a single cell
  • Can profile an unsaved file

@impact27
Copy link
Contributor Author

plot

@impact27 impact27 mentioned this pull request Nov 24, 2021
Merged
3 tasks
@impact27 impact27 mentioned this pull request Dec 29, 2021
Open
10 tasks
@impact27 impact27 mentioned this pull request Jun 11, 2022
Closed
@mrclary
Copy link
Contributor

mrclary commented Jun 11, 2022

@impact27, so this replaces the existing "Run profiler" menu item with "Profile file" and "Profile cell", correct? And the heavy lifting is moved to spyder-kernels so that the profiler is always run in an existing IPython Console environment (inheriting its os.environ, etc.), correct?

If the above is correct, then I agree that this PR should resolve #18010 and obviate PR #18031.

@impact27
Copy link
Contributor Author

@impact27, so this replaces the existing "Run profiler" menu item with "Profile file" and "Profile cell", correct? And the heavy lifting is moved to spyder-kernels so that the profiler is always run in an existing IPython Console environment (inheriting its os.environ, etc.), correct?

If the above is correct, then I agree that this PR should resolve #18010 and obviate PR #18031.

Yes exactly, the profiling would happen in the spyder-kernel, so there would be no difference with runfile and debugfile

@impact27 impact27 mentioned this pull request Jun 12, 2022
Merged
@mrclary
Copy link
Contributor

mrclary commented Jun 12, 2022

@impact27 and @ccordoba12, when will this PR be ready for review? Should this be included in 5.3.2 milestone?

@impact27 impact27 mentioned this pull request Jun 12, 2022
Merged
3 tasks
@impact27 impact27 changed the base branch from 5.x to master June 12, 2022 17:13
@impact27
Copy link
Contributor Author

This is ready for review, assuming the tests pass when they are fixed

@ccordoba12
Copy link
Member

@impact27, the plan for this is to merge first PR #17467, which has a significant refactoring about how things are run in Spyder, then PR #11186 because @andfoy and @steff456 need it to continue their work to move the editor to the new API, then this one.

@ccordoba12 ccordoba12 added this to the v6.0alpha1 milestone Jun 22, 2022
@andfoy
Copy link
Member

andfoy commented Jun 22, 2022

If the IPythonConsole will profile the files, we would prefer to use the viewer run API instead.

@impact27 impact27 force-pushed the profile_script branch 3 times, most recently from 075e4cc to cbbb9e9 Compare June 25, 2022 07:18
Quentin Peter and others added 21 commits September 3, 2024 07:15
fix import
bd87d1f
@ccordoba12 @meeseeksmachine
Backport PR spyder-ide#22459: PR: Update workflows to run in the `6.x…
c9d0eab 
…` branch (CI)
@ccordoba12
Merge pull request spyder-ide#22460 from meeseeksmachine/auto-backpor…
1a2bd38 
…t-of-pr-22459-on-6.x

Backport PR spyder-ide#22459 on branch 6.x (PR: Update workflows to run in the `6.x` branch (CI))

[ci skip]
@meeseeksmachine
Backport PR spyder-ide#22437 on branch 6.x (PR: Fix bug when calling …
61e6fdd 
…`update_edit_menu` at startup (Editor)) (spyder-ide#22439)
@meeseeksmachine
Backport PR spyder-ide#22438 on branch 6.x (PR: Enable autoreload m…
334ce4c 
…agic on all operating systems (Config)) (spyder-ide#22462)
@ccordoba12
Fix development version
a44926b 
That correctly reflects what the next version is going to be.

[ci skip]
@ccordoba12
Merge pull request spyder-ide#22468 from ccordoba12/fix-stable-version
2c8b28f 
PR: Fix development version
@meeseeksmachine
Backport PR spyder-ide#22474 on branch 6.x (PR: Update org.spyder_ide…
7913ca8 
….spyder.appdata.xml) (spyder-ide#22475)
@meeseeksmachine @ccordoba12
Backport PR spyder-ide#22476 on branch 6.x (PR: Fix issues showing th…
b1f96ae 
…e in-app appeal message) (spyder-ide#22477)

Co-authored-by: Carlos Cordoba <[email protected]>
@meeseeksmachine @jitseniesen
Backport PR spyder-ide#22484 on branch 6.x (PR: Change default format…
1b86cb7 
… in array editor to `''`) (spyder-ide#22488)

Co-authored-by: Jitse Niesen <[email protected]>
@meeseeksmachine @ccordoba12
Backport PR spyder-ide#22502 on branch 6.x (PR: Reset `undocked befor…
24bdc68 
…e hiding` state of all plugins before applying layout) (spyder-ide#22503)

Co-authored-by: Carlos Cordoba <[email protected]>
@meeseeksmachine @ccordoba12
Backport PR spyder-ide#22504 on branch 6.x (PR: Don't use script to g…
b2843d1 
…et env vars if Spyder is launched from a terminal (Utils)) (spyder-ide#22506)

Co-authored-by: Carlos Cordoba <[email protected]>
@meeseeksmachine @ccordoba12
Backport PR spyder-ide#22509 on branch 6.x (PR: Don't kill kernel pro…
7284d89 
…cess tree when running in Binder (IPython console)) (spyder-ide#22510)

Co-authored-by: Carlos Cordoba <[email protected]>
@dalthviz @meeseeksmachine
Backport PR spyder-ide#22442: PR: Update contributing, release and ma…
27747d1 
…intenance instructions for backporting
@dalthviz
Merge pull request spyder-ide#22517 from meeseeksmachine/auto-backpor…
c24adcb 
…t-of-pr-22442-on-6.x

Backport PR spyder-ide#22442 on branch 6.x (PR: Update contributing, release and maintenance instructions for backporting)
@dalthviz @meeseeksmachine
Backport PR spyder-ide#22424: PR: Prioritize conda-forge channel so t…
e307e25 
…hat stable releases are pulled above unstable ones (Installers)
@dalthviz
Merge pull request spyder-ide#22518 from meeseeksmachine/auto-backpor…
456adad 
…t-of-pr-22424-on-6.x

Backport PR spyder-ide#22424 on branch 6.x (PR: Prioritize conda-forge channel so that stable releases are pulled above unstable ones (Installers))
@dalthviz @meeseeksmachine
Backport PR spyder-ide#22490: PR: Prevent error when updating `sys.pa…
949e11b 
…th` in consoles (IPython console)
@dalthviz
Merge pull request spyder-ide#22523 from meeseeksmachine/auto-backpor…
ada993e 
…t-of-pr-22490-on-6.x

Backport PR spyder-ide#22490 on branch 6.x (PR: Prevent error when updating `sys.path` in consoles (IPython console))
Merge remote-tracking branch 'upstream/6.x' into profile_script
9cac28a
git subrepo clone (merge) --branch=improve_namespace --force https://…
3fccd35 
…github.com/impact27/spyder-kernels.git external-deps/spyder-kernels

subrepo:
  subdir:   "external-deps/spyder-kernels"
  merged:   "40a123630"
upstream:
  origin:   "https://github.com/impact27/spyder-kernels.git"
  branch:   "improve_namespace"
  commit:   "40a123630"
git-subrepo:
  version:  "0.4.5"
  origin:   "https://github.com/ingydotnet/git-subrepo"
  commit:   "aa416e4"
@impact27 impact27 changed the base branch from master to 6.x September 18, 2024 05:14
@impact27
Copy link
Contributor Author

@ccordoba12 I updated this PR to merge on 6.x but there are no tests running?

@ccordoba12
Copy link
Member

I updated this PR to merge on 6.x but there are no tests running?

Please move it back to master. We changed our development workflow and now everything is merged to master, and only bugfixes are backported (by doing cherry-picks) to 6.x.

And the same goes for Spyder-kernels.

@impact27 impact27 changed the base branch from 6.x to master September 23, 2024 21:11
@ccordoba12
Copy link
Member

ccordoba12 commented Sep 24, 2024 •
edited
Loading

@impact27, now 6.x and master are independent branches, so you can't perform merges between them (that's why your history now shows all backport PRs we created against 6.x, which can't be merged back to master because the code for them is already there).

Do you want me to give you a hand to fix your branch?

@impact27
Copy link
Contributor Author

impact27 commented Sep 25, 2024 •
edited
Loading

Do you want me to give you a hand to fix your branch?

Yes that would be great thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ccordoba12 ccordoba12 ccordoba12 left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@impact27 impact27

Labels
component:Profiler type:Enhancement
Projects
None yet
Milestone
v6.1.0
Development

Successfully merging this pull request may close these issues.

Spyder profiler uses wrong python when using virtualenv
8 participants
@impact27 @pep8speaks @ccordoba12 @mrclary @andfoy @dalthviz @CAM-Gerlach @meeseeksmachine