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

house: results are now nonsense... #1415

Closed
yawpitch opened this issue Jul 9, 2018 · 4 comments
Closed

house: results are now nonsense... #1415

yawpitch opened this issue Jul 9, 2018 · 4 comments

Comments

@yawpitch
Copy link
Contributor

yawpitch commented Jul 9, 2018

#1354 seems to have fixed an apparent inconsistency in test data with the other "songs", but in doing so has made the actual results make no sense.

This:
expected = [
"This is the cat",
"that killed the rat",
"that ate the malt",
"that lay in the house that Jack built."
]

Was updated to this:

expected = [
        "This is the cat"
        "that killed the rat"
        "that ate the malt"
        "that lay in the house that Jack built."
]

But that means that what was returned ["This is the cat", "that killed the rat", "that ate the malt", "that lay in the house that Jack built"] is now ["This is the catthat killed the ratthat ate the maltthat lay in the house that Jack built"] ... now you do indeed have a single sentence, but it's full of nonsense words.

The data may have appeared inconsistent by breaking up sentences, but I would argue that what it actually did was break up stanzas, which have no bearing on sentences. Poems (and songs) are traditionally delivered by breaking sentences up into stanzas, normally along either boundaries of meter or rhyme repetition.

@cmccandless
Copy link
Contributor

Summary

What should have been done in this exercise

Replace newlines with spaces.

What was actually done

Newlines were removed.

@yawpitch
Copy link
Contributor Author

Well, actually, I think I said that newlines were appropriate ... but if you're going to destroy the structure of a poem by removing newlines, at least replace them with spaces.

@cmccandless
Copy link
Contributor

Forgive my previous concise reply. I simply meant that when this exercise was updated to match the canonical data, it was done incorrectly. The canonical data replaced newlines with spaces.

@yawpitch
Copy link
Contributor Author

yawpitch commented Jul 10, 2018 via email

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

No branches or pull requests

2 participants