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

[Currency Exchange]: Reworded task explanations #2626

Merged
merged 8 commits into from
Dec 14, 2021

Conversation

BrooksAz
Copy link
Contributor

@BrooksAz BrooksAz commented Oct 3, 2021

I added some additional explanation to steps 1-6, just a few hints to point the user in the right direction of what operation to use. I managed to figure out steps 5 and 6 and I think the new instructions make it more clear on what to do. I also believe this wording is less overwhelming, breaking things into smaller parts.

I added some additional explanation to steps 1-5, just a few hints to point the user in the right direction of what operation to use. I still don't fully understand 5 and six so they need some more work in my opinion.
@BrooksAz BrooksAz marked this pull request as draft October 3, 2021 23:00
@BrooksAz BrooksAz marked this pull request as ready for review October 3, 2021 23:32
@BrooksAz
Copy link
Contributor Author

BrooksAz commented Oct 4, 2021

#2621 Related to this issue

@J08K
Copy link
Member

J08K commented Oct 4, 2021

Hey @BrooksAz,
I just had a conversation with @BethanyG on this exercise. The fact is that this exercise is in need of a major rework. We think that it is not a good idea to keep re-wording this exercise. I will be posting something in the Slack channel: #maintaining-python about the fact that we'll probably need to rethink the entirety of the exercise. Therefore I will be putting this PR on hold for now. I'll update you on any developments, and do not hesitate to DM me on Slack (@J08K).

Thank you! ❤️

@J08K J08K added the on hold ✋🏽 Action should stop on this issue or PR for now. label Oct 4, 2021
@BrooksAz
Copy link
Contributor Author

BrooksAz commented Oct 4, 2021

@J08K , thank you for the update, I'm happy to have been able to help bring the issue to attention however it is addressed!

@J08K
Copy link
Member

J08K commented Oct 4, 2021

If the rest of the community agrees that this exercise needs a complete re-work, would you like to work on it and give it your own whirl? This would mean starting from scratch and probably choosing a different topic as well. I'll obviously help you along the way if and when needed for any questions.

@BrooksAz
Copy link
Contributor Author

BrooksAz commented Oct 4, 2021

I'd love to look into it!

@BethanyG
Copy link
Member

BethanyG commented Oct 4, 2021

@J08K @BrooksAz -- One thing to think on as we edge toward re-working this further. The original design spec, which was not copied into the design doc (we need to do that!!).

We originally had complex numbers in this mix, because they are a basic numeric type supported by Python. The original authors of this exercise did not find a way to incorporate them.

It would be great if a re-write could -- without too much of a stretch.

Additionally, we may or may not want to relent and use a few things from math() (not too many!!) ...just to ease a few operations.

@BrooksAz
Copy link
Contributor Author

BrooksAz commented Oct 4, 2021

@BethanyG Personally I don't think we should bring such a relatively high level math concept into the same level as basic adding and subtracting, that seems like it would be even more confusing.

@BethanyG
Copy link
Member

BethanyG commented Oct 4, 2021

@BrooksAz -- agree we shouldn't be doing polar coordinates or complicated transforms. But basic math works (with caveat) on complex numbers. And they are a basic numeric type. So we should be including them here, or in a follow-on exercise.

@BrooksAz
Copy link
Contributor Author

BrooksAz commented Oct 4, 2021 via email

@J08K
Copy link
Member

J08K commented Oct 5, 2021

Well, I think that not many people get excited about algebraic homework... So we do have to choose a subject that intrigues people in a way.

@BrooksAz
Copy link
Contributor Author

BrooksAz commented Oct 5, 2021

True true, imaginary numbers are just a little abstract for me to think of a basic enough practical exercise without getting into a large amount of background for those without the prior knowledge.

@BethanyG
Copy link
Member

BethanyG commented Oct 5, 2021

Just putting this Python docs link on numeric types in for reference. The table shows what is allowed and not for different numeric types. It also talks about how complex numbers are represented (both parts are floats). For me, the big takeaway is that (more or less) what works for ints & floats also works for these "things" called complex numbers. Heres also a brief rundown from Tutorials Point that is (less good) but does something similar -- with "just the facts" of what you can & cannot "do".

My contention is that we need not (at this time) go any further than that. This is intended as an exercise on what the built-in numeric types are, and what baseline things are possible to do with them. As a concept exercise, this is meant to focus on the concept of numbers and basic numeric operations -- not on figuring out a novel algorithm or math equation....or in the case of the exercise as it now stands, the ins and outs of currency exchange.

Maybe (if it supports an exercise story) we talk about how complex numbers are used in electrical circuits (without a huge amount of detail, and with clear examples of usage) or to describe translation or rotation of images (again, without too much digression into complex number space, and with clear examples of usage).

Most of this for the student should be about following the examples, and having enough story to engage (without too much confusion) enough to focus on learning the Python literals, syntax and code. We don't want to mis-represent a "real world" problem -- but we also don't want these exercises to become lessons on how to do currency exchange or (to use another exercise) how to write a control system for a nuclear reactor. So somewhere between "dirll-and-kill" and write a whole program. Which can be tough (as you've seen), but not impossible.

My original thinking on this was a sort of numbers branch or series in the syllabus tree. First, the core built-in types (int, float, complex). Then the types that are about re-representing ints (binary, octal, hexadecimal), and then the ones about re-representing floats (decimal, fraction). Finally, we might dig into the math() module, or go for some more direct applications (like polar coordinates or heavy image manipulation).

But first, we need to get numbers sorted. 😄

@BrooksAz
Copy link
Contributor Author

BrooksAz commented Oct 5, 2021 via email

@BethanyG
Copy link
Member

BethanyG commented Oct 5, 2021

@BrooksAz -- No apologies necessary! Its a discussion. 😄 Its just hard (for me) to covey what a concept exercise is, as opposed to the practice exercises, which are much more open-ended.

@kotp kotp marked this pull request as draft October 5, 2021 22:04
@BethanyG BethanyG changed the title Reworded task explanations [Currency Exchange]: Reworded task explanations Oct 8, 2021
@BeerHuntor
Copy link

Ive managed to fix the issues with the discrepancies between the test.py and the main application, but it always fails on the final test. Even with comments.. Ive tried to have a look at the test.py to see if i can make any sense out of it, and get it to run, but i don't understand it. Was going to make a pull request to fix the function name discrepancies, but after reading this thread, i won't bother.

@BethanyG
Copy link
Member

Hi @BeerHuntor,

Sorry to hear you are having issues with this exercise. Maybe if you file an issue with what you have tried, one of us can help you with it? Without seeing where you are stuck, we can't fix the problem. Commenting in the PR without detail isn't terribly helpful.

Ive managed to fix the issues with the discrepancies between the test.py and the main application, but it always fails on the final test. Even with comments.. Ive tried to have a look at the test.py to see if i can make any sense out of it, and get it to run, but i don't understand it.

I am not sure what discrepancies you are referring to, but I'd be happy to explain the tests to you if you'd like to open and issue and show me what you tried, and what errors you are getting. Thanks!

@BeerHuntor
Copy link

BeerHuntor commented Oct 18, 2021

Hi @BeerHuntor,

Sorry to hear you are having issues with this exercise. Maybe if you file an issue with what you have tried, one of us can help you with it? Without seeing where you are stuck, we can't fix the problem. Commenting in the PR without detail isn't terribly helpful.

Ive managed to fix the issues with the discrepancies between the test.py and the main application, but it always fails on the final test. Even with comments.. Ive tried to have a look at the test.py to see if i can make any sense out of it, and get it to run, but i don't understand it.

I am not sure what discrepancies you are referring to, but I'd be happy to explain the tests to you if you'd like to open and issue and show me what you tried, and what errors you are getting. Thanks!

The function names were different from the test than the main.. I believe the one in question was the elapsed_time_in_minutes() function, it was missing the 'in' from the function name, so was throwing test errors, not sure if it was something I did or if it pulled in an incorrect file, And I've managed to fix the test, I assumed it was looking for comments, not a code block.. so that's why it was throwing me failures.

@BethanyG
Copy link
Member

@BeerHuntor -- This is a PR. Please open an issue if you would like to discuss/get help with a problem.

The function name you are referring to belongs to a different exercise, called Guido's Gorgeous Lasagna, a not this exercise, which is called Currency Exchange.

And, AFIK the issue you mention has been fixed in PR 2651.

@BeerHuntor
Copy link

@BeerHuntor -- This is a PR. Please open an issue if you would like to discuss/get help with a problem.

The function name you are referring to belongs to a different exercise, called Guido's Gorgeous Lasagna, a not this exercise, which is called Currency Exchange.

And, AFIK the issue you mention has been fixed in PR 2651.

Apologies, I could of sworn I was on the correct exercise.

@exercism exercism locked as off-topic and limited conversation to collaborators Oct 18, 2021
@BethanyG BethanyG removed the on hold ✋🏽 Action should stop on this issue or PR for now. label Dec 10, 2021
@BethanyG BethanyG requested a review from J08K December 10, 2021 20:23
@BethanyG BethanyG marked this pull request as ready for review December 10, 2021 20:23
@BethanyG
Copy link
Member

@J08K -- I gave this one more look, and made some additional changes. Given the delay in us re-writing this exercise, I figured we could go ahead and do some re-phrasing for clarity. Can you take a last look and approve or close? Thanks so much!

Copy link
Member

@J08K J08K left a comment

Choose a reason for hiding this comment

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

Great work, just a few comments and suggestions. No actual need commit them, but put them there anyway. You can push and rebase if you'd like! 💙

BethanyG and others added 2 commits December 13, 2021 10:35
Co-authored-by: Job van der Wal <48634934+J08K@users.noreply.github.com>
Co-authored-by: Job van der Wal <48634934+J08K@users.noreply.github.com>
@BethanyG BethanyG merged commit 59e95e5 into exercism:main Dec 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants