clulab / processors

Natural Language Processors
https://clulab.github.io/processors/
417 stars 101 forks source link

Update holiday normalization #771

Closed kwalcock closed 6 months ago

kwalcock commented 6 months ago

This appears to be a Java 8 vs. Java 11 problem. At one point I had to update Jenkins to use Java 11, because that is the lowest version supported. It is somehow causing a problem.

kwalcock commented 6 months ago

Some problems are described here: https://github.com/svendiedrichsen/jollyday/issues/88

kwalcock commented 6 months ago

https://github.com/focus-shift/jollyday says that JDK 11 is required.

MihaiSurdeanu commented 6 months ago

How about we pull the plug and remove the corenlp subproject?

kwalcock commented 6 months ago

:-) I may be able to simply shade it. I can also try to use jollyday differently so that it doesn't throw the exception. After all, stanford uses it and those tests aren't failing. Updating to a new corenlp will probably not help, because they use the same old version of jollyday.

MihaiSurdeanu commented 6 months ago

I was serious about removing it. It depends on older Java, introduces many differences, and causes all these conflicts...

kwalcock commented 6 months ago

The holiday tests have passed on Linux with Java 8, 11, 14, 17, 18, on Windows with Java 8, 11, 14, 17, 18 and Mac with java 8, 11. There were other tests that failed intermittently on Mac (8, 11) and Windows (18). They went away when I retested. There's a chance that the dependency change has some kind of transitive effect which will cause users of the library problems, Look here to the cause. Holiday handling could be a lot better and an issue will be filed with some details.

kwalcock commented 6 months ago

Is that a thing?