-
Notifications
You must be signed in to change notification settings - Fork 83
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
documentation: Fix nits on the first marimo notebook #3933
Conversation
_first_info_text = exercise_text( | ||
first_text_area.value, | ||
"first", | ||
((1, 2), (3, 4)), | ||
("first(1, 2) = ", "first(3, 4) = "), | ||
) |
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.
Our ruff formatting excludes notebooks, so it's my local settings I assume.
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.
Are you editing the raw python locally or using marimo edit?
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.
Raw python locally, do you want me to run with the editor? I'll see if it can reformat these.
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.
Yeah it would be good to run it in the editor, before merging because they have their own formatter that will likely conflict with your changes.
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.
Yeah, there is no "format all cells", I have to go over each cell and do a ctrl-b
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.
"Run all cells" seems to do it
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3933 +/- ##
=======================================
Coverage 91.30% 91.30%
=======================================
Files 467 467
Lines 58068 58068
Branches 5581 5581
=======================================
Hits 53021 53021
Misses 3616 3616
Partials 1431 1431 ☔ View full report in Codecov by Sentry. |
|
||
A module is a unit of code which holds a single region. |
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.
Regions are not explained, so my main issue here is adding a definition earlier or omitting all together.
I'm inclined to do that later.
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.
This is great, thank you
Minor nits and rewording of sentences.