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

fix(curriculum): description improvement case converter program #54051

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

larymak
Copy link
Contributor

@larymak larymak commented Mar 11, 2024

Checklist:

Closes: #54232
Closes: #53359

@github-actions github-actions bot added the scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. label Mar 11, 2024
@larymak larymak requested a review from zairahira March 25, 2024 07:35
@zairahira
Copy link
Member

The changes are good to go from my end. 🎉

@zairahira zairahira added the status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. label Mar 25, 2024
@moT01
Copy link
Member

moT01 commented Mar 26, 2024

If this is ready, you can take it out of draft mode @larymak. There should be a button below that says "ready for review" you can click.

@larymak larymak marked this pull request as ready for review March 26, 2024 05:12
@larymak larymak force-pushed the feat/case-converter-program branch 3 times, most recently from c6396b8 to 873b86b Compare March 28, 2024 09:58
Copy link
Contributor

@Supravisor Supravisor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In step 12

To display the output, enclose the fucntion call within the print() function.

Replace fucntion with function.

@zairahira zairahira added new python course and removed status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. labels Apr 8, 2024
@zairahira zairahira removed the status: waiting update To be applied to PR if a maintainer/reviewer has left a feedback and follow up is needed from OP label Apr 16, 2024
@Sembauke Sembauke added status: waiting update To be applied to PR if a maintainer/reviewer has left a feedback and follow up is needed from OP and removed status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. labels Apr 16, 2024
Copy link
Member

@zairahira zairahira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎉

@zairahira zairahira added status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. and removed status: waiting update To be applied to PR if a maintainer/reviewer has left a feedback and follow up is needed from OP labels Apr 21, 2024
Copy link
Contributor

@Dario-DC Dario-DC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think some important changes are required to clarify how to work with list comprehensions:

  • step 17
    I'd leave the snake_cased_char_list list empty for this step. Let's add the
    return ''.join(snake_cased_char_list).strip('_') here to add interactivity, so the campers can actually see what happens.
  • step 18
    snake_cased_char_list becomes
    ['_' + char.lower() for char in pascal_or_camel_cased_string]
  • step 19
    snake_cased_char_list = ['_' + char.lower() for char in pascal_or_camel_cased_string if char.isupper()]
  • step 20
    snake_cased_char_list = [ '_' + char.lower() if char.isupper() else char for char in pascal_or_camel_cased_string ]

In this way:

  • we add a bit of interactivity
  • we write comprehensions that work in each step
  • we teach the difference between having an if and an if/else (which is quite annoying)

@naomi-lgbt naomi-lgbt added status: waiting update To be applied to PR if a maintainer/reviewer has left a feedback and follow up is needed from OP and removed status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. labels Apr 25, 2024
@larymak
Copy link
Contributor Author

larymak commented Apr 26, 2024

I think some important changes are required to clarify how to work with list comprehensions:

  • step 17
    I'd leave the snake_cased_char_list list empty for this step. Let's add the
    return ''.join(snake_cased_char_list).strip('_') here to add interactivity, so the campers can actually see what happens.
  • step 18
    snake_cased_char_list becomes
    ['_' + char.lower() for char in pascal_or_camel_cased_string]
  • step 19
    snake_cased_char_list = ['_' + char.lower() for char in pascal_or_camel_cased_string if char.isupper()]
  • step 20
    snake_cased_char_list = [ '_' + char.lower() if char.isupper() else char for char in pascal_or_camel_cased_string ]

In this way:

  • we add a bit of interactivity
  • we write comprehensions that work in each step
  • we teach the difference between having an if and an if/else (which is quite annoying)

Need more clarity on this

@Dario-DC
Copy link
Contributor

Need more clarity on this

We should change the following step:

  • step 17
    Ask the camper to add return ''.join(snake_cased_char_list).strip('_') so in the following steps we can see something that is not None printed on the screen.

seed code

def convert_to_snake_case(pascal_or_camel_cased_string):
    # snake_cased_char_list = []
    # for char in pascal_or_camel_cased_string:
    #     if char.isupper():
    #       converted_character = '_' + char.lower()
    #       snake_cased_char_list.append(converted_character)
    #     else:
    #         snake_cased_char_list.append(char)
    # snake_cased_string = ''.join(snake_cased_char_list)
    # clean_snake_cased_string = snake_cased_string.strip('_')

    # return clean_snake_cased_string

    snake_cased_char_list = [ ]
--fcc-editable-region--

--fcc-editable-region--
def main():
    print(convert_to_snake_case('aLongAndComplexString'))

if __name__ == '__main__':
    main()
  • step 18
    Go back to the snake_cased_char_list list. Teach how a basic list comprehension works and ask the campers to turn the empty list into ['_' + char.lower() for char in pascal_or_camel_cased_string].

seed code

def convert_to_snake_case(pascal_or_camel_cased_string):
    # snake_cased_char_list = []
    # for char in pascal_or_camel_cased_string:
    #     if char.isupper():
    #       converted_character = '_' + char.lower()
    #       snake_cased_char_list.append(converted_character)
    #     else:
    #         snake_cased_char_list.append(char)
    # snake_cased_string = ''.join(snake_cased_char_list)
    # clean_snake_cased_string = snake_cased_string.strip('_')

    # return clean_snake_cased_string
--fcc-editable-region--
    snake_cased_char_list = [ ]
--fcc-editable-region--
    return ''.join(snake_cased_char_list).strip('_')

def main():
    print(convert_to_snake_case('aLongAndComplexString'))

if __name__ == '__main__':
    main()
  • step 19
    Now the output is wrong because we are doing the same thing for every character. Teach how to add an if to the comprehension and ask the camper do it. At the end of this step we should have
    snake_cased_char_list = ['_' + char.lower() for char in pascal_or_camel_cased_string if char.isupper()]

seed code

def convert_to_snake_case(pascal_or_camel_cased_string):
    # snake_cased_char_list = []
    # for char in pascal_or_camel_cased_string:
    #     if char.isupper():
    #       converted_character = '_' + char.lower()
    #       snake_cased_char_list.append(converted_character)
    #     else:
    #         snake_cased_char_list.append(char)
    # snake_cased_string = ''.join(snake_cased_char_list)
    # clean_snake_cased_string = snake_cased_string.strip('_')

    # return clean_snake_cased_string
--fcc-editable-region--
    snake_cased_char_list = ['_' + char.lower()  for char in pascal_or_camel_cased_string]
--fcc-editable-region--
    return ''.join(snake_cased_char_list).strip('_')

def main():
    print(convert_to_snake_case('aLongAndComplexString'))

if __name__ == '__main__':
    main()
  • step 20
    The output is still wrong because now the comprehension is doing something only with uppercase characters. Teach how to write if/else inside the comprehension. At the end of this step we should have
    snake_cased_char_list = [ '_' + char.lower() if char.isupper() else char for char in pascal_or_camel_cased_string ]

seed code

def convert_to_snake_case(pascal_or_camel_cased_string):
    # snake_cased_char_list = []
    # for char in pascal_or_camel_cased_string:
    #     if char.isupper():
    #       converted_character = '_' + char.lower()
    #       snake_cased_char_list.append(converted_character)
    #     else:
    #         snake_cased_char_list.append(char)
    # snake_cased_string = ''.join(snake_cased_char_list)
    # clean_snake_cased_string = snake_cased_string.strip('_')

    # return clean_snake_cased_string
--fcc-editable-region--
    snake_cased_char_list = ['_' + char.lower()  for char in pascal_or_camel_cased_string if char.isupper()]
--fcc-editable-region--
    return ''.join(snake_cased_char_list).strip('_')

def main():
    print(convert_to_snake_case('aLongAndComplexString'))

if __name__ == '__main__':
    main()
  • step 21 and 22 remain the same

@larymak
Copy link
Contributor Author

larymak commented May 1, 2024

Need more clarity on this

We should change the following step:

  • step 17
    Ask the camper to add return ''.join(snake_cased_char_list).strip('_') so in the following steps we can see something that is not None printed on the screen.

seed code

def convert_to_snake_case(pascal_or_camel_cased_string):
    # snake_cased_char_list = []
    # for char in pascal_or_camel_cased_string:
    #     if char.isupper():
    #       converted_character = '_' + char.lower()
    #       snake_cased_char_list.append(converted_character)
    #     else:
    #         snake_cased_char_list.append(char)
    # snake_cased_string = ''.join(snake_cased_char_list)
    # clean_snake_cased_string = snake_cased_string.strip('_')

    # return clean_snake_cased_string

    snake_cased_char_list = [ ]
--fcc-editable-region--

--fcc-editable-region--
def main():
    print(convert_to_snake_case('aLongAndComplexString'))

if __name__ == '__main__':
    main()
  • step 18
    Go back to the snake_cased_char_list list. Teach how a basic list comprehension works and ask the campers to turn the empty list into ['_' + char.lower() for char in pascal_or_camel_cased_string].

seed code

def convert_to_snake_case(pascal_or_camel_cased_string):
    # snake_cased_char_list = []
    # for char in pascal_or_camel_cased_string:
    #     if char.isupper():
    #       converted_character = '_' + char.lower()
    #       snake_cased_char_list.append(converted_character)
    #     else:
    #         snake_cased_char_list.append(char)
    # snake_cased_string = ''.join(snake_cased_char_list)
    # clean_snake_cased_string = snake_cased_string.strip('_')

    # return clean_snake_cased_string
--fcc-editable-region--
    snake_cased_char_list = [ ]
--fcc-editable-region--
    return ''.join(snake_cased_char_list).strip('_')

def main():
    print(convert_to_snake_case('aLongAndComplexString'))

if __name__ == '__main__':
    main()
  • step 19
    Now the output is wrong because we are doing the same thing for every character. Teach how to add an if to the comprehension and ask the camper do it. At the end of this step we should have
    snake_cased_char_list = ['_' + char.lower() for char in pascal_or_camel_cased_string if char.isupper()]

seed code

def convert_to_snake_case(pascal_or_camel_cased_string):
    # snake_cased_char_list = []
    # for char in pascal_or_camel_cased_string:
    #     if char.isupper():
    #       converted_character = '_' + char.lower()
    #       snake_cased_char_list.append(converted_character)
    #     else:
    #         snake_cased_char_list.append(char)
    # snake_cased_string = ''.join(snake_cased_char_list)
    # clean_snake_cased_string = snake_cased_string.strip('_')

    # return clean_snake_cased_string
--fcc-editable-region--
    snake_cased_char_list = ['_' + char.lower()  for char in pascal_or_camel_cased_string]
--fcc-editable-region--
    return ''.join(snake_cased_char_list).strip('_')

def main():
    print(convert_to_snake_case('aLongAndComplexString'))

if __name__ == '__main__':
    main()
  • step 20
    The output is still wrong because now the comprehension is doing something only with uppercase characters. Teach how to write if/else inside the comprehension. At the end of this step we should have
    snake_cased_char_list = [ '_' + char.lower() if char.isupper() else char for char in pascal_or_camel_cased_string ]

seed code

def convert_to_snake_case(pascal_or_camel_cased_string):
    # snake_cased_char_list = []
    # for char in pascal_or_camel_cased_string:
    #     if char.isupper():
    #       converted_character = '_' + char.lower()
    #       snake_cased_char_list.append(converted_character)
    #     else:
    #         snake_cased_char_list.append(char)
    # snake_cased_string = ''.join(snake_cased_char_list)
    # clean_snake_cased_string = snake_cased_string.strip('_')

    # return clean_snake_cased_string
--fcc-editable-region--
    snake_cased_char_list = ['_' + char.lower()  for char in pascal_or_camel_cased_string if char.isupper()]
--fcc-editable-region--
    return ''.join(snake_cased_char_list).strip('_')

def main():
    print(convert_to_snake_case('aLongAndComplexString'))

if __name__ == '__main__':
    main()
  • step 21 and 22 remain the same

Changes incorporated, share feedback

@zairahira zairahira requested a review from Dario-DC May 1, 2024 11:34
@zairahira zairahira added status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. and removed status: waiting update To be applied to PR if a maintainer/reviewer has left a feedback and follow up is needed from OP labels May 1, 2024
Copy link
Contributor

@ilenia-magoni ilenia-magoni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there should be some improvements

Comment on lines 19 to 23
const transformedCode = code.replace(/\r/g, "");
const convert_to_snake_case = __helpers.python.getDef("\n" + transformedCode, "convert_to_snake_case");
const { function_body } = convert_to_snake_case;

assert.match(function_body, / +snake_cased_char_list\s*=\s*\[\s*("|')_\1\s*\+\s*char\.lower\(\s*\)\s+if\s+char\.isupper\(\s*\)\s*\]/);
assert.match(function_body, /return\s*''\.join\(snake_cased_char_list\)\.strip\('_'\)/);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use AST tests when we are creating new ones, it allows any equivalent code, irrespective of spaces and quotes used and stuff.

Suggested change
const transformedCode = code.replace(/\r/g, "");
const convert_to_snake_case = __helpers.python.getDef("\n" + transformedCode, "convert_to_snake_case");
const { function_body } = convert_to_snake_case;
assert.match(function_body, / +snake_cased_char_list\s*=\s*\[\s*("|')_\1\s*\+\s*char\.lower\(\s*\)\s+if\s+char\.isupper\(\s*\)\s*\]/);
assert.match(function_body, /return\s*''\.join\(snake_cased_char_list\)\.strip\('_'\)/);
({
test: () => assert(runPython(`
_Node(_code).find_function('convert_to_snake_case').find_return().is_equivalent('return "".join(snake_cased_char_list).strip('_'))
`))
})

Comment on lines 21 to 25
const transformedCode = code.replace(/\r/g, "");
const convert_to_snake_case = __helpers.python.getDef("\n" + transformedCode, "convert_to_snake_case");
const { function_body } = convert_to_snake_case;

assert.match(function_body, / +snake_cased_char_list\s*=\s*\[\s*'_'\s*\+\s*char\.lower\(\s*\)\s+if\s+char\.isupper\(\s*\)\s*else\s*char\s*for\s+char\s+in\s+pascal_or_camel_cased_string\s*\]/);
}
})
assert.match(function_body, /if\s*char\.isupper\(\)/);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test passes on the seed code.

Copy link
Contributor

@Dario-DC Dario-DC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't able to leave suggestions in some steps, so I left simple comments.

Some example of list comprehension syntax are required. I wrote the first examples that came to my mind but we can write something more meaningful.

@@ -7,26 +7,22 @@ dashedName: step-19

# --description--

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--description--

List comprehensions accept conditional statements, to evaluate the provided expression only if certain condition are met:

spam = [i * 2 for i in iterable if i > 0]

As you can see from the output, the list of characters generated from pascal_or_camel_cased_string has been joined. Since the expression inside the list comprehension is evaluated for each character, the result is a lowercase string with all the characters separated by an underscore.

Follow the example above to add an if clause to your list comprehension so that the expression is executed only if the character is uppercase.

--hints--

You should add and if clause with the condition char.isupper() to your list comprehension.

({ test: () => assert(runPython(`
ifs = _Node(_code).find_function("convert_to_snake_case").find_variable("snake_cased_char_list").find_comp_ifs()
len(ifs) == 1 and ifs[0].is_equivalent("char.isupper()")
`)) })

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can consider to add another test to check the existing part of the comprehension.

@zairahira zairahira added status: waiting update To be applied to PR if a maintainer/reviewer has left a feedback and follow up is needed from OP and removed status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. labels May 6, 2024
@larymak larymak requested a review from a team as a code owner May 24, 2024 09:17
@github-actions github-actions bot added the scope: i18n language translation/internationalization. Often combined with language type label label May 24, 2024
---
id: 663b10c10a4c0a0e095137ee
title: Step 18
challengeType: 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
challengeType: 0
challengeType: 20

({
test: () => assert(runPython(`
_Node(_code).find_function('convert_to_snake_case').find_return().is_equivalent('return "".join(snake_cased_char_list)')
`));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`));
`))

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JFYI .find_return().is_equivalent("return x") works fine, but if you want you can use .has_return("x") which does the same thing under the hood.

---
id: 663b16e62fee463b4caf46e9
title: Step 19
challengeType: 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
challengeType: 0
challengeType: 20

```js
({
test: () => assert(runPython(`
_Node(_code).find_function("convert_to_snake_case").find_variable("snake_cased_char_list").finfind_comp_expr()[0].is_equivalent("'_' + char.lower()")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_Node(_code).find_function("convert_to_snake_case").find_variable("snake_cased_char_list").finfind_comp_expr()[0].is_equivalent("'_' + char.lower()")
_Node(_code).find_function("convert_to_snake_case").find_variable("snake_cased_char_list").find_comp_expr().is_equivalent("'_' + char.lower()")

The using the return statement, return an empty string `''` joined with `snake_cased_char_list` as the argument, and strip any leading or trailing underscores from the result.
Before proceeding to work on the list comprehension, you're going to give your function a return value. In this way you'll be able to check the output.

Using the the return statement, return the list `snake_cased_char_list`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Using the the return statement, return the list `snake_cased_char_list`.
Use the `return` statement to return the list `snake_cased_char_list` from your function.


After returning the list `snake_cased_char_list`, you will need to join its elements into a single string using an empty string `''` as the separator.

Modify the return statement to return the list `snake_cased_char_list` joined with an empty string as a separator.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Modify the return statement to return the list `snake_cased_char_list` joined with an empty string as a separator.
Modify the `return` statement to return the result of joining `snake_cased_char_list` with an empty string as a separator.


# --hints--

You should modify the return statement to join the `snake_cased_char_list` using the `join` method with an empty string `''` as the separator.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
You should modify the return statement to join the `snake_cased_char_list` using the `join` method with an empty string `''` as the separator.
You should modify the `return` statement to return the result of joining `snake_cased_char_list` with an empty string `''` as the separator using the `.join()` method.

---

# --description--

List comprehensions are more than just transformation, they can also filter elements based on conditions using `if` statements. You can add an `if` statement within a list comprehension to filter elements based on a given condition.
List comprehensions accept conditional statements, to evaluate the provided expression only if certain condition are met:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
List comprehensions accept conditional statements, to evaluate the provided expression only if certain condition are met:
List comprehensions accept conditional statements, to evaluate the provided expression only if certain conditions are met:


# --hints--

You should add `if` statement after the `for` clause within the square braces of `snake_cased_char_list` to check if `char` is uppercase.
You should add and `if` clause with the condition `char.isupper()` to your list comprehension.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
You should add and `if` clause with the condition `char.isupper()` to your list comprehension.
You should add an `if` clause with the condition `char.isupper()` to your list comprehension.


# --description--

After joining the elements of the list `snake_cased_char_list`, you will need to remove any leading or trailing underscores from the resulting string. For this, use the `strip` method with the underscore character `_` as an argument.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably we should explain explicitly that method calls can be chained and how it should be done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new python course scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. scope: i18n language translation/internationalization. Often combined with language type label status: waiting update To be applied to PR if a maintainer/reviewer has left a feedback and follow up is needed from OP
Projects
None yet
8 participants