-
Notifications
You must be signed in to change notification settings - Fork 368
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
Intro Level Tutorial #590
Intro Level Tutorial #590
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't proofread any of the actual tutorials yet, but I've got a few comments on the organization and structure, to start with:
docs/intro.rst
Outdated
@@ -0,0 +1,23 @@ | |||
Introduction |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's rename merge this file and "contents.rst" into one file. We've already got the "overview.html" page as an intro to Memray, so what we need here is just an overview of the structure and contents of the tutorials, what prerequisites they have, how they're structured, etc. Less "intro to Memray", more "intro to these tutorials".
Something like:
The tutorials in this section of the docs provide a gentle introduction to debugging memory issues using the Memray profiler. You'll learn how to use basic features like (...) as well as some more advanced features like (...). There are 3 different tutorials: ... You should follow them in the order they're presented here, as each builds on concepts introduced by the last.
Etc etc. We just want to explain how to use the tutorials, and what they are, and what they do (and maybe don't) cover, and what you'll learn by using them.
Also, the merged file should be called "tutorials.rst" or "tutorials/index.rst" or something like that (the filename becomes part of the URL, so it'll matter for links).
I think I most like the idea of pushing all of these .rst
files down into the docs/tutorials
, and having them as docs/tutorials/index.rst
(which should become accessible as either https://bloomberg.github.io/memray/tutorials or as https://bloomberg.github.io/memray/tutorials/index.html), and then having the other tutorial pages be in that same folder. At which point, https://bloomberg.github.io/memray/tutorials/exercise_1.html won't be a terrible name, though I wonder if https://bloomberg.github.io/memray/tutorials/1.html or https://bloomberg.github.io/memray/tutorials/fibonacci.html might be better.
docs/index.rst
Outdated
|
||
.. toctree:: | ||
:hidden: | ||
:caption: Hands-on Tutorial | ||
|
||
intro | ||
contents | ||
exercise_1 | ||
exercise_2 | ||
exercise_3 | ||
additional_features |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's actually yank this up higher - I think I'd like to create a new section called "Concepts" and drop run
through performance
into it, and then lift this section up to between getting_started
and run
. That way the sidebar will be ordered as:
- Overview
- Getting started
- Hands-on Tutorial
- About these tutorials
- Exercise 1 - ...
- Exercise 2 - ...
- Exercise 3 - ...
- What to learn about next
- Concepts
- run
- python_allocators
...
I think that's a pretty nice organization, and should do a good job of highlighting these new docs. We should also make getting_started.html suggest that the next steps after you've generated your first flame graph would be to either try the tutorials or to learn more about `memray run`, with links to either the tutorial or skipping over it to the concepts section.
@godlygeek I have addressed your comments for restructuring, as well as combining the introduction to the tutorials. I think we should still update the rst page for additional features inside the tutorials directory. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #590 +/- ##
==========================================
+ Coverage 92.55% 92.70% +0.14%
==========================================
Files 91 92 +1
Lines 11304 11234 -70
Branches 1581 2055 +474
==========================================
- Hits 10462 10414 -48
+ Misses 837 820 -17
+ Partials 5 0 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
70e3bca
to
24dace0
Compare
I've now updated a couple internal links that stopped working due to the new tutorials/ directory. Otherwise looks great-- thanks for the fixes while I was out @jcarnaxide ! I've also added a brief section that encourages to explore the @godlygeek we should be good for another look whenever you have the time -- thank you! |
Signed-off-by: Gintare Statkute <statkute.g@gmail.com>
Signed-off-by: Jesse Carnaxide <jcarnaxide@gmail.com>
Pull tutorial `.rst` files down into separate folder. Also, reorganize the side bar. Signed-off-by: Jesse Carnaxide <jcarnaxide@gmail.com>
Signed-off-by: Gintare Statkute <gstatkute@bloomberg.net>
Update the tutorials to follow our docs' typical typographical conventions, improve some wording here and there, use American spellings rather than British spellings for consistency with the rest of our documentation, and avoid using any more inline HTML than necessary for handling expandable spans for solutions. Signed-off-by: Matt Wozniski <godlygeek@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made some edits, after which this LGTM.
I'm going to land this, so that it can be used by our Sprinters at PyCon US tomorrow.
@jcarnaxide and @statkute please take a look at the last commit on this PR if you get a chance, and open an issue or a new PR if you spot any mistakes I've introduced. Sorry for not giving you time to review before I landed it!
Issue number of the reported bug or feature request: #
Describe your changes
We have worked on an introductory level course for people to get familiar with Memray.
Testing performed
We ran
make docs-live
to view the new RST files, and to see everything is rendered properly.We also ran the exercises via instructions specified and was able to get through the whole tutorial.
Additional context
It is worth noting, that I plan to update the instructions to include running the tutorial via codespaces, but we stuck with simple venv/pip for this first PR.