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

Doubled commas when two appositives are next to each other #15

Closed
jeremybmerrill opened this issue Jul 3, 2015 · 4 comments · Fixed by #18
Closed

Doubled commas when two appositives are next to each other #15

jeremybmerrill opened this issue Jul 3, 2015 · 4 comments · Fixed by #18

Comments

@jeremybmerrill
Copy link
Contributor

In every year when a condition occured, since 1999,, except 2001, the Yankees have lost the World Series.

SimpleNLG erroneously generates two commas after "1999" above. Both "since 1999" and "except 2001" are appositives, each added to the prepositional phrase "In every year when a condition occurred"

Here's the code that generates it, using a fairly simple Ruby library that interfaces with Java and SimpleNLG:

require 'simplernlg' if RUBY_PLATFORM == 'java'
NLG = SimplerNLG::NLG

subj = NLG.factory.create_noun_phrase('the', "Yankees")
main_clause = {
  :s => subj,
  :number => 'plural',
  :v => "lost", 
  :perfect => true,
  :tense => :present,
  :o => "the World Series",
}
sentence = NLG.phrase(main_clause)

condition_phrase = NLG.phrase({
                          :s => "a condition",
                          :v => "occur",
                          :tense => :past
                      })
condition_phrase.set_feature(NLG::Feature::COMPLEMENTISER, 'when')
year_np = NLG.factory.create_noun_phrase('every', 'year' )
year_np.add_complement(condition_phrase)
prep_phrase = NLG.factory.create_preposition_phrase('in', year_np)

since_pp = NLG.factory.create_preposition_phrase('since', NLG.factory.create_noun_phrase('1999'))
since_pp.set_feature(NLG::Feature::APPOSITIVE, true) # responsible for commas 1 and either 2 or 3

prep_phrase.add_post_modifier(since_pp)

except_phrase = NLG.factory.create_preposition_phrase('except', NLG.factory.create_noun_phrase('2001') )
except_phrase.set_feature(NLG::Feature::APPOSITIVE, true) # responsible for comma 2 or 3
prep_phrase.add_post_modifier(except_phrase)

sentence.add_front_modifier(prep_phrase) 

NLG.realizer.setCommaSepCuephrase(true) # responsible for comma 4
puts NLG.realizer.realise_sentence(sentence) 

# output, with commas numbered
# In every year when a condition occured, since 1999,, except 2001, the Yankees have lost the World Series.
#                                       1           23            4

Obviously this isn't a difficult bug to solve in my client code (I can just replace all doubled commas with a single comma), but probably better for SimpleNLG to handle it on its own.

I'm working on getting the other bug (#13) replicated with code for you...

@rdeoliveira
Copy link
Contributor

Jeremy, I guess a good starting point to fix this bug would be OrthographyProcessor.java from line 184 onwards.

@jeremybmerrill
Copy link
Contributor Author

Thanks, Rodrigo! I'll take a look.

I've made a handful of modifications:

  • to add commas around appositive pre-modifiers: 6abc16a
  • to comma-separate all cue phrases and front modifiers, if commaSepcuephrase is set: jeremybmerrill@5d1252e
  • and to make the COMPLEMENTISER feature final, which I think is necessary to allow it to be set (at least in JRuby): 3eed77f

all of the tests pass with this modifications and they enable some sentence structures for me. Would you be interested in pull requests?

@Saad-Mahamood
Copy link
Member

Jeremy, thanks for contributing your fixes. I have looked at the code changes and they look fine to me. Before I merge, would it be possible to write two JUnit tests as well for the first two code changes in the OrthographyFormatTests class?

@jeremybmerrill
Copy link
Contributor Author

Sure, I'll give that a shot shortly.

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 a pull request may close this issue.

3 participants