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

Add test for Parenthesized Context Mgr #52

Merged
merged 5 commits into from
Mar 22, 2024

Conversation

brocla
Copy link
Contributor

@brocla brocla commented Mar 11, 2024

@BethanyG This PR has two code snippets that exercise the parenthesis feature for context managers (new in 3.10).

I couldn't find an example directly from a student exercise, so I created this one that opens two files with a single context manager. The open statements are wrapped in parens, within the context manager.

The second snippet is copied from the python docs. Even though this doesn't look like a student exercise, I like it because it succinctly goes through many variations of the parens.

  - this is a new python feature in 3.10

This comment was marked as resolved.

@github-actions github-actions bot closed this Mar 11, 2024
@BethanyG BethanyG reopened this Mar 11, 2024
@BethanyG BethanyG self-requested a review March 11, 2024 18:09
Copy link
Member

@BethanyG BethanyG left a comment

Choose a reason for hiding this comment

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

The second snippet is copied from the python docs. Even though this doesn't look like a student exercise, I like it because it succinctly goes through many variations of the parens.

Completely agree that having examples of the variations is really good. Buuut, I am a little resistant to using a long example with a bunch of pass statements. Let me think on it a bit, and see if I can cobble together something that has a bit more meat on it, but isn't too tedious with the variations.

I could grab one of the concept exercise test cases, but those are rather kludgy as well. Or maybe an example using Decimal.localcontext().

@brocla
Copy link
Contributor Author

brocla commented Mar 20, 2024

I just realized that I don't like my last commit. Please ignore it. I'll replace soonest.
Kevin

@brocla
Copy link
Contributor Author

brocla commented Mar 20, 2024

OK. Better.
I have removed the offending example and replaced it with a golden-like example. It uses the localcontext context manager twice, each with a different style of parens.

The example with the context manager that open two file simultaneously remains.

Kevin

Copy link
Member

@BethanyG BethanyG left a comment

Choose a reason for hiding this comment

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

Nice work, you! Thanks for putting this together.

I have a small qualm about the pandas code, since we don't have pandas installed on any of our tooling. However, the code parses just fine (it just wouldn't run if we tried evaluating it) -- so I think we can go ahead an merge this. 😄

@BethanyG BethanyG merged commit 5d25828 into exercism:main Mar 22, 2024
1 check passed
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.

2 participants