-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add spiral-matrix #1170
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
Add spiral-matrix #1170
Conversation
config.json
Outdated
"algorithms", | ||
"control-flow", | ||
"lists", | ||
"logic" |
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 don't think that logic
really applies to this exercise. A better example would be the zebra-puzzle
exercise.
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.
Good point, @cmccandless. I decided to delete "logic" from the list of topics.
|
||
from spiral_matrix import spiral | ||
|
||
|
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.
Please add a comment referencing the current version of the canonical data. For the format of this comment, please see #784.
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.
Done. Thanks for the hint.
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.
Done. Made the suggested change.
config.json
Outdated
"algorithms", | ||
"control-flow", | ||
"lists", | ||
"logic" |
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.
Good point, @cmccandless. I decided to delete "logic" from the list of topics.
|
||
from spiral_matrix import spiral | ||
|
||
|
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.
Done. Thanks for the hint.
@chgraef Please add a reference to #752 to the PR description (see magic words). |
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.
There's one very minor change and then I think this is ready to be merged.
|
||
from spiral_matrix import spiral | ||
|
||
# Tests adapted from `problem-specifications//canonical-data.json` @ v1.1.0 |
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.
Can you move this line down by one to be more consistent with the other test files?
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.
Thanks, @N-Parsons ,will do.
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 believe for full consistency, you the spacing should be 2 blank lines above, one below (see below):
from spiral_matrix...
# Tests adapted from...
class...
@chgraef Merged; thanks for working on this! |
[Docs] Add reference to required reading in implementing guide
[Docs] Add reference to required reading in implementing guide
[Docs] Add reference to required reading in implementing guide
[Docs] Add reference to required reading in implementing guide
Implementing exercise spiral-matrix. Closes #752