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 exercises on objects #114

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

yakutovicha
Copy link
Member

@yakutovicha yakutovicha commented May 5, 2023

fixes #111

Comment on lines +38 to +40
test_solution = [string for _, string in function_to_test(flavors)]
reference_solution = [string for _, string in reference_ice_cream_scoop(flavors)]
assert test_solution == reference_solution
Copy link
Member

Choose a reason for hiding this comment

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

I've been thinking about this test all afternoon (mostly). And it can be faked easily because it doesn't really enforce you to define/use the class. One can build a tuple with (None, "<Properly formatted string>") and the test will pass.

I think it's trickier that it seems to perform some checks on the actual class. One way is to parse the AST as Simone did with some FP tests, but I didn't want to implement it for the time being.

Needs some more thoughts... 💭

Copy link
Member

Choose a reason for hiding this comment

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

You could check whether there is a class with the name Scoop in the locals() of the function. Something like:

import inspect
import ast

def check_if_class_in_scope(class_name: str, scope: callable) -> bool:
    source = ast.parse(inspect.getsource(scope))
    for node in ast.walk(source):
        match node:
            case ast.ClassDef() as cd:
                return cd.name == class_name


def f1():
    class A:
        a = 1


is_a = check_if_class_in_scope("A", f1)  # True
is_b = check_if_class_in_scope("B", f1)  # True
print(f"A is {is_a}, B is {is_b}")

Copy link
Member

Choose a reason for hiding this comment

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

That's a good suggestion 👍🏻 We must change the solution and add the class inside the solution function. Or apply the same idea to the entire code of the cell.

@yakutovicha yakutovicha marked this pull request as draft June 21, 2023 14:22
Comment on lines +38 to +40
test_solution = [string for _, string in function_to_test(flavors)]
reference_solution = [string for _, string in reference_ice_cream_scoop(flavors)]
assert test_solution == reference_solution
Copy link
Member

Choose a reason for hiding this comment

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

You could check whether there is a class with the name Scoop in the locals() of the function. Something like:

import inspect
import ast

def check_if_class_in_scope(class_name: str, scope: callable) -> bool:
    source = ast.parse(inspect.getsource(scope))
    for node in ast.walk(source):
        match node:
            case ast.ClassDef() as cd:
                return cd.name == class_name


def f1():
    class A:
        a = 1


is_a = check_if_class_in_scope("A", f1)  # True
is_b = check_if_class_in_scope("B", f1)  # True
print(f"A is {is_a}, B is {is_b}")

@edoardob90 edoardob90 changed the title Fix exercises on objects. Fix exercises on objects Jul 14, 2023
@edoardob90
Copy link
Member

Don't know (yet) if we want to fix this issue before the next tutorial, but here's something we can do without changing anything substantially: add a test on the type (e.g. isinstance) of the return value of the solution function instead of only its content.

At the moment, we are only testing the content with test_solution = [string for _, string in function_to_test(flavors)]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix exercises 1-3 on OOP and their tests
3 participants