Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Fix exercises on objects#114

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
yakutovicha wants to merge2 commits intomain
base:main
Choose a base branch
Loading
fromfix/exercises-on-objects
Draft

Conversation

yakutovicha
Copy link
Member

@yakutovichayakutovicha commentedMay 5, 2023
edited
Loading

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 nameScoop in the locals() of the function. Something like:

importinspectimportastdefcheck_if_class_in_scope(class_name:str,scope:callable)->bool:source=ast.parse(inspect.getsource(scope))fornodeinast.walk(source):matchnode:caseast.ClassDef()ascd:returncd.name==class_namedeff1():classA:a=1is_a=check_if_class_in_scope("A",f1)# Trueis_b=check_if_class_in_scope("B",f1)# Trueprint(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 classinside the solution function. Or apply the same idea to the entire code of the cell.

@yakutovichayakutovicha marked this pull request as draftJune 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 nameScoop in the locals() of the function. Something like:

importinspectimportastdefcheck_if_class_in_scope(class_name:str,scope:callable)->bool:source=ast.parse(inspect.getsource(scope))fornodeinast.walk(source):matchnode:caseast.ClassDef()ascd:returncd.name==class_namedeff1():classA:a=1is_a=check_if_class_in_scope("A",f1)# Trueis_b=check_if_class_in_scope("B",f1)# Trueprint(f"A is{is_a}, B is{is_b}")

@edoardob90edoardob90 changed the titleFix exercises on objects.Fix exercises on objectsJul 14, 2023
@edoardob90
Copy link
Member

Don't know (yet) if we want to fix this issuebefore the next tutorial, but here's something we can do without changing anything substantially: add a test on thetype (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 withtest_solution = [string for _, string in function_to_test(flavors)]

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@edoardob90edoardob90edoardob90 left review comments

@baffellibaffellibaffelli requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix exercises 1-3 on OOP and their tests
3 participants
@yakutovicha@edoardob90@baffelli

[8]ページ先頭

©2009-2025 Movatter.jp