Modernize with immutable Document and Sentence

[kwalcock]

No description provided.

[kwalcock]

Please ignore the individual commits and instead see file differences. This updates the project to be compatible with the current LTS (long term support) version of Scala, which is 3.3.6, and its conventions. It sacrifices some compatibility with older versions in that it insists that both Document and Sentence be immutable. In this version they have no vars or exposed Arrays. The Seqs are usually implemented by ArraySeq, which wraps the underlying array for read-only access. (A Scala3+ version would have an IArray here for efficiency.) The most invasive changes are in BalaurProcessor (including some remaining TODOs) and then the numerous updates from Array to Seq in method signatures. There were very few algorithmic changes needed, but they should be regression tested. There are many more potential changes, especially if the Stanford stack no longer needs to be supported. I resisted splitting Sentence into TokenizedSentence and AnnotatedSentence versions, but that would be a logical next step.

[kwalcock]

One disadvantage is that access to the Scala Seqs from Java is more complicated than it is to Array. This shows especially in JavaProcessorsExample.scala (https://github.com/clulab/processors/blob/kwalcock/processors2b/apps/src/main/java/org/clulab/processors/apps/ProcessorsJavaExample.java)

[MihaiSurdeanu]

This looks great. Thank you @kwalcock !
Is the switch from Array to Seq to enforce immutability?

[kwalcock]

@MihaiSurdeanu, it is partly to enforce it (immutability), but almost just as much to document it. Scala 2.13 and 3 compel one to do this by not allowing without significant complaint and making copies of the arrays in cases like

val seq: Seq[Int] = Array(1, 2)

which is what all the implicit conversions in e.g. https://github.com/clulab/processors/tree/kwalcock/processors2b/library/src/main/scala-3/org/clulab/scala are meant to avoid. This update uses the newer ArraySeq from Scala 2.13 and Scala 3 to handle the issue and backports to Scala 2 using scala-collection-compat. A more thorough update would make the other implicit conversions unnecessary, but that's too big a step just now.

[kwalcock]

Here is a worrisome TODO.

[MihaiSurdeanu]

Yes, I don't know where that's from... Is any code using this variable? It may be a weird leftover... If Reach or Habitus do not require it, I'm Ok removing it.

[kwalcock]

Below is the complete code. A Sentence is temporarily created in order to call lexiconNer.find(sentence). That generally only uses the lemmas or words through LexiconNER.getTokens which never accesses the raw field anyway. However, the normal constructor for a Sentence is raw, starts, ends, words.

processors/library/src/main/scala/org/clulab/processors/clu/BalaurProcessor.scala

Lines 207 to 223 in e986420

	private def mkNerLabelsOpt(
	words: Seq[String], startOffsets: Seq[Int], endOffsets: Seq[Int],
	tags: Seq[String], lemmas: Seq[String]
	): Option[Seq[String]] = {
	lexiconNerOpt.map { lexiconNer =>
	val sentence = Sentence(
	words, // TODO: Why isn't this raw?
	startOffsets,
	endOffsets,
	words,
	Some(tags),
	Some(lemmas)
	)
	lexiconNer.find(sentence)
	}
	}

[MihaiSurdeanu]

I vaguely remember this... I think some lexicon NER dictionaries operate over lemmas rather than words, which required such weird work arounds...

[kwalcock]

I'd like to do something better here.

[MihaiSurdeanu]

@MihaiSurdeanu, it is partly to enforce it (immutability), but almost just as much to document it. Scala 2.13 and 3 compel one to do this by not allowing without significant complaint and making copies of the arrays in cases like

val seq: Seq[Int] = Array(1, 2)

which is what all the implicit conversions in e.g. https://github.com/clulab/processors/tree/kwalcock/processors2b/library/src/main/scala-3/org/clulab/scala are meant to avoid. This update uses the newer ArraySeq from Scala 2.13 and Scala 3 to handle the issue and backports to Scala 2 using scala-collection-compat. A more thorough update would make the other implicit conversions unnecessary, but that's too big a step just now.

Thank you!

[MihaiSurdeanu]

Thank you @kwalcock !

[kwalcock]

The ColumnsToDocument code included at least two places where it is implied that a function manipulates a sentence or document:

setLabels(s, labels.toArray)

val d = new Document(sentences.toArray)
annotate(d)

That doesn't work anymore, so the function signatures were changed. ColumnsToDocument is in the apps subproject even though it isn't an application. It is not used anywhere in the project that I can find. Is it then library code that some client is expected to make use of? Perhaps it should be moved to the library subproject.

[MihaiSurdeanu]

The ColumnsToDocument code included at least two places where it is implied that a function manipulates a sentence or document:

setLabels(s, labels.toArray)

val d = new Document(sentences.toArray)
annotate(d)

That doesn't work anymore, so the function signatures were changed. ColumnsToDocument is in the apps subproject even though it isn't an application. It is not used anywhere in the project that I can find. Is it then library code that some client is expected to make use of? Perhaps it should be moved to the library subproject.

I used it I think when I created the training data for the parser. That can be easily adjusted.