NL and AGF handling #143

FlxB2 · 2018-07-03T13:30:55Z

Most stuff is finished. Trees are able to be generated from the content of @grammar annotations (which are now called AGF - Amy Grammar Format - expressions) and checked if valid. The generated trees can also be transformed in valid JSGF rules. Right now i'm working on the NLParser, which categorizes the strings received from the web or speech recognition. The last step will be a NLInterpreter class which returns the right matching grammar.
Feel free to suggest changes.

AGF grammars are specified in the wiki

close #95
fixed #177

…rst implementation of the lexer

# Conflicts: # core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/speech/SpeechCommandHandler.java

buddy200

In itself, quite understandable. However, general things could be better described in the class Comments

buddy200 · 2018-07-03T15:33:24Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/agf/AGFLexer.java

+import java.util.List;
+
+/**
+ * AGF lexer implementation


I think to improve understanding in general it would be better to make more detailed Java class comments. (Not only in this class)

buddy200 · 2018-07-03T15:35:44Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/agf/AGFParser.java

+
+/**
+ * 
+ * the AGF Parser implementation


same as above

buddy200 · 2018-07-03T15:37:57Z

...rc/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/agf/parselets/IAGFParselet.java

+import de.unistuttgart.iaas.amyassist.amy.core.natlang.agf.nodes.AGFNode;
+
+/**
+ * AGF Parselet interface


same as above

buddy200 · 2018-07-03T15:40:28Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/JSGFGenerator.java

+				for(int i = 0; i < node.getChilds().size(); i++) {
+					handleNode(b, node.getChilds().get(i));
+					if(i!=node.getChilds().size()-1) {
+						b.append("|");


maybe it makes sense to pack this into a method. I think it is to much code for a case statement

buddy200 · 2018-07-03T15:41:37Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/JSGFGenerator.java

+				for(int i = 0; i < node.getChilds().size(); i++) {
+					handleNode(b, node.getChilds().get(i));
+					if(i!=node.getChilds().size()-1) {
+						b.append("|");


added deprecations added NLProcessingManager

Amy-Bot

SonarQube analysis found issues:
Bugs: 0
Vulnerabilities: 0
Code Smells: 27

See all issues in SonarCloud

Amy-Bot · 2018-07-04T10:44:10Z

...tuttgart/iaas/amyassist/amy/core/speech/util/NaturalLanguageInterpreterAnnotationReader.java

@@ -37,6 +37,7 @@
 * 
 * @author Leon Kiefer
 */
+@Deprecated
 public class NaturalLanguageInterpreterAnnotationReader {


Code Smell: Add the missing @deprecated Javadoc tag. (squid:MissingDeprecatedCheck)

See it in SonarCloud

Amy-Bot · 2018-07-04T10:44:10Z

api/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/plugin/api/Grammar.java

@@ -38,7 +39,9 @@
 @Target(java.lang.annotation.ElementType.METHOD)
 public @interface Grammar {
 	/**
-	 * @return the grammar
+	 * TODO description


Code Smell: Complete the task associated to this TODO comment. (squid:S1135)

See it in SonarCloud

Amy-Bot · 2018-07-04T10:44:10Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/NLProcessingManagerImpl.java

+	@Override
+	public String process(String naturalLanguageText) {
+		this.logger.debug("input {}", naturalLanguageText);
+		NLLexer nlLexer = new NLLexer(naturalLanguageText);


Code Smell: Remove this useless assignment to local variable "nlLexer". (squid:S1854)

See it in SonarCloud

Amy-Bot · 2018-07-04T10:44:10Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/JSGFGenerator.java

+	 *            The keyword
+	 */
+	@Deprecated
+	public void addRule(String ruleName, String[] keywords, String grammar) {


Code Smell: Add the missing @deprecated Javadoc tag. (squid:MissingDeprecatedCheck)

See it in SonarCloud

Amy-Bot · 2018-07-04T10:44:10Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/NLProcessingManagerImpl.java

+import de.unistuttgart.iaas.amyassist.amy.core.io.Environment;
+import de.unistuttgart.iaas.amyassist.amy.core.natlang.agf.AGFLexer;
+import de.unistuttgart.iaas.amyassist.amy.core.natlang.agf.AGFParser;
+import de.unistuttgart.iaas.amyassist.amy.core.natlang.agf.AGFToken;


Code Smell: Remove this unused import 'de.unistuttgart.iaas.amyassist.amy.core.natlang.agf.AGFToken'. (squid:UselessImportCheck)

See it in SonarCloud

Amy-Bot · 2018-07-04T10:44:13Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/nl/NLParser.java

+	/**
+	 * contains regex with the corresponding WordTokenType
+	 */
+	Map<String, WordTokenType> regexTokenType = new HashMap<>();{


Code Smell: Move the contents of this initializer to a standard constructor or to field initializers. (squid:S1171)

See it in SonarCloud

Amy-Bot · 2018-07-04T10:44:13Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/NLProcessingManagerImpl.java

+				.isAnnotationPresent(de.unistuttgart.iaas.amyassist.amy.core.plugin.api.SpeechCommand.class)) {
+			throw new IllegalArgumentException();
+		}
+		String[] speechKeyword = NLIAnnotationReader.getSpeechKeywords(natuaralLanguageInterpreter);


Code Smell: Remove this useless assignment to local variable "speechKeyword". (squid:S1854)

See it in SonarCloud

Amy-Bot · 2018-07-04T10:44:13Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/PartialNLI.java

+			Object[] params = { input };
+			return (String) this.method.invoke(instance, params);
+		} catch (IllegalAccessException | InvocationTargetException e) {
+			throw new RuntimeException("Tryed to invoke " + this.method.getName(), e);


Code Smell: Define and throw a dedicated exception instead of using a generic one. (squid:S00112)

See it in SonarCloud

Amy-Bot · 2018-07-04T10:44:13Z

api/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/plugin/api/SpeechCommand.java

@@ -27,7 +27,8 @@
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
-
+//TODO move the class in a package in the natlang package


Code Smell: Complete the task associated to this TODO comment. (squid:S1135)

See it in SonarCloud

Amy-Bot · 2018-07-04T10:44:13Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/speech/SpeechCommandHandler.java

@@ -51,14 +51,16 @@
 * @author Leon Kiefer
 */
 @Service
+@Deprecated
 public class SpeechCommandHandler {


Code Smell: Add the missing @deprecated Javadoc tag. (squid:MissingDeprecatedCheck)

See it in SonarCloud

# Conflicts: # core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/speech/result/handler/SpeechCommandHandler.java # core/src/test/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/TestNLIAnnotationReader.java

cleanup tests moved completeSetup to GrammarObjectsCreator

neumantm · 2018-07-14T14:41:16Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/JSGFGenerator.java

+				+ "nine | ten | eleven | twelve | thirteen | fourteen | fifteen | "
+				+ "sixteen | seventeen | eighteen | nineteen | twenty | thirty | forty | "
+				+ "fifty | sixty  | seventy | eighty | ninety | and )+; \n");
+


Is there a way to do this, so the number 'and' is not allowed?

it's saver for the speech recognition, i could make another rule for "and" but that would be kind of pointless

neumantm · 2018-07-14T14:42:16Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/JSGFGenerator.java

+		grammar.append("\n//pre defined rules \n");
+
+		// pre defined rules
+		grammar.append("<digit> = (one | two | three | four | five | six | seven |"


These are not digits but numers, because of the '+' at the end.

neumantm · 2018-07-14T14:43:41Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/JSGFGenerator.java

+	public String addRule(AGFNode node, String ruleName) {
+		StringBuilder b = new StringBuilder();
+		b.append("public <" + ruleName + "> = ");
+		String rule = handleNode(b, node) + ";\n";


Code duplicate: Use private method publicRule().

neumantm · 2018-07-14T14:51:04Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/LanguageSpecifics.java

+		try {
+			return Integer.valueOf(numberInt);
+			}catch(NumberFormatException e) {
+				this.logger.error(String.format("number in numbers file in wrong format %s" , numberInt));


The logger already has the ability to add parameters into the string.

neumantm · 2018-07-14T14:53:22Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/LanguageSpecifics.java

+			}catch(NumberFormatException e) {
+				this.logger.error(String.format("number in numbers file in wrong format %s" , numberInt));
+			}
+		return Integer.MAX_VALUE;


Does this make sense? If not it would be better to just rethrow a IllegalStateException.

neumantm · 2018-07-14T14:55:52Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/NLIAnnotationReader.java

+	 * 
+	 * @param method
+	 *            the method that should be a NLIMethod
+	 */


Add @throws in JavaDoc.

… error state

neumantm · 2018-07-16T12:28:36Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/JSGFGenerator.java

+		grammar.append("<number> = (one | two | three | four | five | six | seven |"
+				+ "nine | ten | eleven | twelve | thirteen | fourteen | fifteen | "
+				+ "sixteen | seventeen | eighteen | nineteen | twenty | thirty | forty | "
+				+ "fifty | sixty  | seventy | eighty | ninety | and )+; \n");


Only allow and if there is number before and behind.

neumantm · 2018-07-16T12:40:49Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/LanguageSpecifics.java

+		Map<String, Integer> result = new HashMap<>();
+		String[] stringNmbRep;
+
+		InputStream grammarFile = this.getClass().getResourceAsStream("englishNumbers.natlang");


Use getClass().getClassLoader().getResourceAsStream()

it strangely only works without .getClassLoader() now, no idea why

the ClassLoader have a absolute scope and relative paths don't work

neumantm · 2018-07-16T12:42:41Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/LanguageSpecifics.java

+				BufferedReader bufferedReader = new BufferedReader(inputStreamReader)) {
+
+			// first line contains all numbers seperated by ',' and ending with ';'
+			String s = bufferedReader.readLine();


You could also have one rule in one line and iterate over all lines.

neumantm · 2018-07-16T12:44:03Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/NLIAnnotationReader.java

+
+	/**
+	 * 
+	 * calls the given method and handles handles possible exceptions


neumantm · 2018-07-16T12:44:13Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/NLIAnnotationReader.java

+	}
+
+	/**
+	 * 


Remove empty line

neumantm · 2018-07-16T16:43:00Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/NLProcessingManagerImpl.java

+
+	private final List<PartialNLI> register = new ArrayList<>();
+
+	private final List<AGFNode> cachedNodeList = new ArrayList<>();


This name is not describing the use very good. "cached" sounds like this list is not necessary, just for speed improvements.

neumantm · 2018-07-16T16:53:10Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/agf/AGFLexer.java

+    			//only allow normal SPACE codepoint 32, U+0020
+    			case 32: 
+    				return this.next();
+				default:


Wrong indentation.

neumantm · 2018-07-16T16:58:24Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/agf/AGFLexer.java

+
+			} 		
+		}
+		throw new NoSuchElementException("thrown by AGFLexer");


Where it is thrown, is automatically contained in this exception. If no good reason can be stated, use constructor without parameter.

neumantm · 2018-07-16T16:59:25Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/agf/AGFLexerException.java

+	/**
+	 * serial version uid
+	 */
+	private static final long serialVersionUID = 1L;


Generate svuid, don't use 1L.

neumantm · 2018-07-16T17:14:36Z

core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/agf/Parser.java

+	}
+
+	/**
+	 * 


add method description.

FlxB2 added 11 commits June 26, 2018 13:03

added the natlang and natlang.agf package, some interfaces and the fi…

daf740f

…rst implementation of the lexer

added first steps for the parser

b199e53

added first steps for the parser

06af78b

removed unnecessary interface

5230072

added optional groups and forbids a number following another number

6cab908

some cleanup

d0abc7b

added parse to jsgf expression and changed some things in the parser

2fdfee9

added tests for JSGF Generator and for AGF Trees

ce401aa

added NLLexer and some tests

165f63e

build fix

0640266

NLLexer is now iterator

13aa395

FlxB2 requested review from buddy200 and Legion2 July 3, 2018 13:31

FlxB2 mentioned this pull request Jul 3, 2018

Natural Language to Plugin Function #95

Closed

4 tasks

FlxB2 and others added 3 commits July 3, 2018 15:54

removed GrammarParser and completed JSGFGenerator class

6e86c39

readded deprecated methods, fixed tests and removed some code smells

a45915d

Merge remote-tracking branch 'origin/dev' into NLandAGFparser

33a3d98

# Conflicts: # core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/speech/SpeechCommandHandler.java

Legion2 changed the title ~~NL and AGF handling~~ WIP: NL and AGF handling Jul 3, 2018

buddy200 reviewed Jul 3, 2018

View reviewed changes

Legion2 and others added 4 commits July 3, 2018 18:50

added breaking todos

5828d9e

added deprecations added NLProcessingManager

changes things mentioned in buddy200's review

acf5352

AGFLexer is now implements Iterator<AGFToken>

b3d9aaf

added NLParser and a single test - more to come

973fa7b

Amy-Bot reviewed Jul 4, 2018

View reviewed changes

Legion2 assigned FlxB2 Jul 4, 2018

Legion2 added 5 commits July 6, 2018 14:02

Merge remote-tracking branch 'origin/dev' into NLandAGFparser

f52964c

# Conflicts: # core/src/main/java/de/unistuttgart/iaas/amyassist/amy/core/speech/result/handler/SpeechCommandHandler.java # core/src/test/java/de/unistuttgart/iaas/amyassist/amy/core/natlang/TestNLIAnnotationReader.java

fixed typo

0a06c64

replaced SpeechCommandHandler with NLProcessingManager

69a0737

cleanup tests moved completeSetup to GrammarObjectsCreator

fixed grammar generation

e74d7e2

removed deprecated code

6dabd03

neumantm reviewed Jul 14, 2018

View reviewed changes

neumantm suggested changes Jul 14, 2018

View reviewed changes

fixed broken merge

ce70595

sonarqubecloud bot deleted a comment from FlxB2 Jul 14, 2018

sonarqubecloud bot deleted a comment from Legion2 Jul 14, 2018

sonarqubecloud bot deleted a comment from FlxB2 Jul 14, 2018

Legion2 and others added 8 commits July 14, 2018 17:44

moved resource to the correct package

5f79286

turned logging into exception throwing, because logging don't fix the…

e68830e

… error state

fixed JavaDoc comments

8a5e84e

resolved loop in loop problem

c17cfd8

changed name of custom rule in jsgf grammar from <digit> to <number>

f832a67

removed duplicate code in jsgf generator

e2565c8

fixed javadoc's

457a87a

Merge branch 'dev' into NLandAGFparser

48b2852

Legion2 changed the title ~~WIP: NL and AGF handling~~ NL and AGF handling Jul 14, 2018

FlxB2 added 2 commits July 15, 2018 17:49

Merge branch 'dev' into NLandAGFparser

b54ce76

Merge branch 'dev' into NLandAGFparser

80f621e

neumantm suggested changes Jul 16, 2018

View reviewed changes

FlxB2 added 4 commits July 16, 2018 21:32

fixed some things mentioned in @neumantm's review

a6ec0c5

LanguageSpecifics iterates over every line

4749b78

removed and from jsgf, this will be re added in the next sprint

df7c628

forgot unused variable

c2562d8

Legion2 approved these changes Jul 16, 2018

View reviewed changes

sonarqubecloud bot deleted a comment from FlxB2 Jul 16, 2018

AmyAssist deleted a comment from FlxB2 Jul 17, 2018

neumantm approved these changes Jul 17, 2018

View reviewed changes

Merge branch 'dev' into NLandAGFparser

27e0a23

FlxB2 merged commit 52e2d55 into dev Jul 17, 2018

FlxB2 deleted the NLandAGFparser branch July 17, 2018 08:24

+              	}
+              	/**
+              	 *


		private final List<PartialNLI> register = new ArrayList<>();

		private final List<AGFNode> cachedNodeList = new ArrayList<>();

NL and AGF handling #143

NL and AGF handling #143

Conversation

FlxB2 commented Jul 3, 2018 • edited by Legion2 Loading

buddy200 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Amy-Bot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FlxB2 commented Jul 3, 2018 •

edited by Legion2

Loading