Skip to content

Conversation

@kottmann
Copy link
Member

@kottmann kottmann commented Feb 6, 2017

No description provided.

@kottmann kottmann changed the title [WIP] OPENNLP-975: Add format support for CoNLL-U format OPENNLP-975: Add format support for CoNLL-U format Feb 7, 2017
@kottmann kottmann force-pushed the OPENNLP-975 branch 2 times, most recently from 6dd2a1a to 28d53b7 Compare February 7, 2017 12:23
try {
return new ConlluLemmaSampleStream(new ConlluStream(inFactory), tagset);
}
catch (IOException e) {
Copy link
Member

Choose a reason for hiding this comment

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

cosmetic: move the catch line to prevous line following }

catch (IOException e) {
// That will throw an exception
CmdLineUtil.handleCreateObjectStreamError(e);
return null;
Copy link
Member

Choose a reason for hiding this comment

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

this can be moved out of catch{} as the final return from the method

} catch (IOException e) {
// That will throw an exception
CmdLineUtil.handleCreateObjectStreamError(e);
return null;
Copy link
Member

Choose a reason for hiding this comment

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

move this outside catch {} as final return

String line;
while ((line = reader.readLine()) != null) {
// comment line, skip it
if (line.trim().startsWith("#")) {
Copy link
Member

@smarthi smarthi Feb 7, 2017

Choose a reason for hiding this comment

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

invert the if condition and hence avoid continue;

Change to if (!line.trim().startsWith("#")) { wordLines.add(new ...) }


ConlluTagset tagset;

if ("u".equals(params.getTagset())) {
Copy link
Member

Choose a reason for hiding this comment

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

can this be replaced by a switch stmt? and avoid f-then-else

tagset = ConlluTagset.X;
}
else {
// unkown type
Copy link
Member

Choose a reason for hiding this comment

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

this could go in as default condition in a switch stmt

* @param tagset the type of tag to retrieve, either universial (u) or language specific (x)
*/
public String getPosTag(ConlluTagset tagset) {
if (ConlluTagset.U.equals(tagset)) {
Copy link
Member

Choose a reason for hiding this comment

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

use switch stmt instead ?

@kottmann
Copy link
Member Author

kottmann commented Feb 7, 2017

Yeah, using switch makes it much nicer because I don't have to call equals by hand.

@asfgit asfgit merged commit 740b6e3 into apache:master Feb 7, 2017
@kottmann kottmann deleted the OPENNLP-975 branch February 9, 2017 10:33
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.

3 participants