Skip to content

scale-generator: use flat and sharp symbols#1942

Merged
SaschaMann merged 1 commit intomainfrom
scale-generator
Feb 9, 2022
Merged

scale-generator: use flat and sharp symbols#1942
SaschaMann merged 1 commit intomainfrom
scale-generator

Conversation

@wolf99
Copy link
Copy Markdown
Contributor

@wolf99 wolf99 commented Feb 1, 2022

This PR switches hash symbols (#) to the sharp symbols () and use of b to the flat symbol () in the description.md.

This seems more "musically" accurate, or more domain-correct.
On the other hand, it causes a divergence between the description.md and the canonical-data.json.

I thought about updating the canonical-data.json also, but that may cause issues for any language for which UTF-8 is non-trivial.

What do you think?

@wolf99 wolf99 force-pushed the scale-generator branch 2 times, most recently from d2a4243 to f452d19 Compare February 1, 2022 21:45
Copy link
Copy Markdown
Member

@petertseng petertseng left a comment

Choose a reason for hiding this comment

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

Interesting. there are a few ways I imagine dealing with that discrepancy.

  • At the problem-specifications level, we mention that tests will represent ♯ with # and ♭ with b. For tracks that want to actually use ♯ and ♭, they'll have to delete that text (we don't currently have a good mechanism for deleting text from a problem-specifications-sourced description, only adding)
  • At the problem-specifications level, we mention that tests may represent ♯ with # and ♭ with b. The ambiguity may be a little frustrating as now you don't know whether the tests are actually doing so without looking.
  • At the problem-specifications level, merge this PR as-is. At the track level, if a track wishes to keep # and b, they insert a comment in the test file and/or description saying "for simplicity, we will represent ♯ with # and ♭ with b". If they instead wish to update their tests to use ♯ and ♭, they just do so.
  • There may be other ways I did not think of.

@ErikSchierboom
Copy link
Copy Markdown
Member

The discrepancy is indeed interesting. With Exercism v3, we now support a instructions.append.md file to append text to, so tracks can have an appended message stating that they opt to encode the flats and sharps as regular ascii characters. That said, currently all tracks woukd have to add that message, so that's a downside.

Option 3 is probably the cleanest, while option 2 is more of a compromise but would lead to less contradictory tests vs instructions.

@petertseng
Copy link
Copy Markdown
Member

petertseng commented Feb 2, 2022

Another question. Is this scenario a possibility and concern, and if so, what can be done about it?

  • A track chooses to start using ♯ and ♭ in their tests
  • A student of this track doesn't realise these characters are in any way different from # and b, and in implementing a solution, uses # and b.
  • Tests fail. Student frustrated.

Copy link
Copy Markdown
Member

@petertseng petertseng left a comment

Choose a reason for hiding this comment

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

Upon further reflection, I believe I came up with an answer to my earlier question. I think my suggestion to a track that does choose to use ♯ and ♭ in their tests would be that they can clearly indicate (perhaps in their stubs, so that the student is sure to see it) that the specific characters ♯ and ♭ are being used.

In that case, I think we have a path forward, with the third option I had presented being taken:

At the problem-specifications level, merge this PR as-is. At the track level, if a track wishes to keep # and b, they insert a comment in the test file and/or description saying "for simplicity, we will represent ♯ with # and ♭ with b". If they instead wish to update their tests to use ♯ and ♭, they just do so.

Copy link
Copy Markdown
Member

@ErikSchierboom ErikSchierboom left a comment

Choose a reason for hiding this comment

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

Upon further reflection, I believe I came up with an answer to my earlier question. I think my suggestion to a track that does choose to use ♯ and ♭ in their tests would be that they can clearly indicate (perhaps in their stubs, so that the student is sure to see it) that the specific characters ♯ and ♭ are being used.

I think this can work well enough. Tracks could also consider adding a note to the tests, just to be safe.

@SaschaMann SaschaMann merged commit 4f5155a into main Feb 9, 2022
@SaschaMann SaschaMann deleted the scale-generator branch February 9, 2022 11:55
ErikSchierboom added a commit to ErikSchierboom/problem-specifications that referenced this pull request Feb 11, 2022
* Format using prettier (exercism#1917)

Format using prettier

* updated description of anagrams exercise (exercism#1928)

* updated description of anagrams

* changed anagram description to be one-sentence-per-line

* updated description of anagrams to use sets

* Update Licence

Give a look at the discussion in BR exercism#1930

* rational-numbers: test to reduce abs value (exercism#1938)

* Change saddle point references to row, column (exercism#1948)

* word-search: Add test case

* Update exercises/word-search/canonical-data.json

Agreed.

Co-authored-by: Erik Schierboom <erik_schierboom@hotmail.com>

* meetup: improve descriptions by saying why each case is tested (exercism#1919)

descriptions show whether a date is the first, last, or an arbitrary
middle date of the week. This helps understand why certain cases are
selected.

Closes exercism#974

* word-search: Add cases checking for concatenation and wrapping

The author of this commit thinks that concatenation is highly unlikely,
but the wrapping might be useful to check in languages that allow
negative indices.

* `flatten-array` Add additional test cases (exercism#1953)

* Add additional test cases to flatten-array

* Update exercises/flatten-array/canonical-data.json

Co-authored-by: Peter Tseng <petertseng@users.noreply.github.com>

Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
Co-authored-by: Peter Tseng <petertseng@users.noreply.github.com>

* Fix bowling game copy (exercism#1955)

Fixes exercism#1954

* Add action to format code (exercism#1941)

* build(deps): bump DavidAnson/markdownlint-cli2-action (exercism#1952)

Bumps [DavidAnson/markdownlint-cli2-action](https://github.com/DavidAnson/markdownlint-cli2-action) from 5.0.0 to 5.1.0.
- [Release notes](https://github.com/DavidAnson/markdownlint-cli2-action/releases)
- [Commits](DavidAnson/markdownlint-cli2-action@b3c3b40...744f913)

---
updated-dependencies:
- dependency-name: DavidAnson/markdownlint-cli2-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Reduced rational nr. should be in standard form. (exercism#1958)

* Reduced rational should be in standard form.

The current instructors fail to mention that a reduced rational number should always be rendered in standard form (without any negative value at the denominator).

* remove superflous blank lines; fix wording

* scale-generator: use flat and sharp symbols (exercism#1942)

* Update configlet part in README (exercism#1949)

Co-authored-by: ee7 <45465154+ee7@users.noreply.github.com>

* phone number: only one problem per test input (exercism#1959)

* [Phone Number] Only one problem per test input

Because the area code is not allowed to start with 0 or 1, inputs designed to elicit other errors should not use area codes that start with either of those digits.

* Respect immutability

* Correct field name: s/comment/comments/

* Comments should contain a list.

* Allow prettier to improve comments

* book-store: reorder keys

* darts: reorder keys

* grade-school: reorder keys

* hamming: reorder keys

* high-scores: reorder keys

* largest-series-product: reorder keys

* list-ops: reorder keys

* luhn: reorder keys

* triangle: reorder keys

* scale-generator: reorder keys

* saddle-points: reorder keys

* diffie-hellman: reorder keys

* collatz-conjecture: reorder keys

* anagram: reorder keys

* accumulate: reorder keys

* Add CI script to check correct order of keys

Co-authored-by: Bart Massey <bart.massey@gmail.com>
Co-authored-by: y8l <72172663+yusufadell@users.noreply.github.com>
Co-authored-by: Ivan Ivanov <mr.mstranger@gmail.com>
Co-authored-by: Damian C. Rossney <dcr8898@users.noreply.github.com>
Co-authored-by: mariohuq <mariohuq@github.com>
Co-authored-by: mariohuq <mario.huq@gmail.com>
Co-authored-by: Peter Tseng <petertseng@users.noreply.github.com>
Co-authored-by: Peter Tseng <pht24@cornell.edu>
Co-authored-by: AH WEI <88667293+kwchang0831@users.noreply.github.com>
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
Co-authored-by: Cedd Burge <ceddlyburge@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Davide Alberto Molin <davide@davidemolin.com>
Co-authored-by: wolf99 <281700+wolf99@users.noreply.github.com>
Co-authored-by: June <12543047+junedev@users.noreply.github.com>
Co-authored-by: ee7 <45465154+ee7@users.noreply.github.com>
Co-authored-by: Leah Hanson <astrieanna@gmail.com>
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.

5 participants