optimaize / language-detector

Language Detection Library for Java
Apache License 2.0
567 stars 165 forks source link

Compatibility with cybozu #30

Closed jpbottaro closed 8 years ago

jpbottaro commented 9 years ago

Hi guys,

We loved what you did with optimaize and would love to use it in our library. Unfortunately the current version does not play well with Cybozu. This is a problem for us since many products in our company use Cybozu directly or transitively, so by depending on our library things would break, and migrating all of them at once is not feasible.

The problem is mainly with classes that share the same name/package (see [1] for an example stack trace). So the question is: Would you guys accept a PR that fixes this? It would be a backwards incompatible change as there would be name changes of public classes.

Thanks

[1] Optimaize tries to use a method that does not exist in the class with the same name in Cybozu.

Exception in thread "main" java.lang.NoSuchMethodError: com.cybozu.labs.langdetect.util.LangProfile.getFreq()Ljava/util/Map;
    at be.frma.langguess.LangProfileReader.read(LangProfileReader.java:76)
    at be.frma.langguess.LangProfileReader.read(LangProfileReader.java:48)
    at com.optimaize.langdetect.profiles.LanguageProfileReader.read(LanguageProfileReader.java:27)
    at com.optimaize.langdetect.profiles.LanguageProfileReader.readAll(LanguageProfileReader.java:154)
    ...
fabiankessler commented 9 years ago

Hello JP,

Yes I see your point and agree with a change.

We had left the code we did not touch in the packages as they were. I did not want to "rebrand" everything.

We started off with version number 0.1 even though it already was a functional software. This was to indicate that "things can change". Also, upgrading from 0.4 to the latest release 0.5 requires some code changes from the user.

Although we try to prevent breaking changes, it's too early to make ugly hacks just to be compatible.

What renaming do you have in mind? I vote against renaming classes. It's confusing, and worst case you run into a conflict if the original software does the same. I'd like to have the class path changed. One option is to get rid of the "com.cybozu" and "be.frma" class paths and move them to "com.optimaize". Another is to rename those class paths, eg to "com.cybozu.somethingelse.labs...".

Looking at your stack trace, as a user, I would prefer seeing all under the same class path. Then it's clear it's from the same library.

It could be com.optimaize.cybozu... but long term that doesn't make sense either. Sooner or later things will get changed. In my opinion, it's enough to leave the credits of the former authors in the class headers and on the project's overview page. That's my opinion.

I would prefer if the classes were put somewhere that makes the most sense now, and is most stable (so that they don't need to be moved again).

It would be nice if you could leave your arguments here, and once we decide what's best (maybe others have opinions too or concerns) then you could submit a PR.

The wider this library is adopted, the better for for all of us.

Thanks!

dennis97519 commented 9 years ago

I've encountered this as well. I think the problem is that you had the original Shuyo's lang detect library installed. This optimaize library comes with an edited version/different version of that library. Just remove the langdetect.jar or something. Although the package name all have com.cybozu.labs it terminates in different names. In this case it is com.cybozu.labs.langdetect.

I'm quite sure you(@jpbottaro) have the old library installed. I solved this nosuchmethoderror by removing the langdetect.jar from the dependencies. btw maybe you(@fabiankessler) should update the readme to remind people dont install the cybozu lab langdetect library.

But I too support the integration of the code, i.e. not use the old profile converter but directly generate the new profile and everything, and rewrite the code if needed. (will be a long process since some part of the code are apparently not understood by you(@fabiankessler) lol)

dennis97519 commented 9 years ago

Or if your company intend to use cybozu's langdetect concurrently with optimaize's language-detection (I don't see the point of doing so though :p, language-detection itself is based on langdetect) you may have to reformat one of the sources yourself to get it working.

janhoy commented 9 years ago

I think you should rename the packages com.cybozu and be.frma into your own namespace com.optimaize so they don't clash with jars from the original projects or forks thereof.

However much we wish that people stick to one library, larger projects will sooner or later include multiple dependencies, which in turn might include language detection as one feature, and the resulting class path may then (outside the control of the main application author) contain conflicting libraries.

fabiankessler commented 9 years ago

Nice, thanks! I'll wait a bit (in case there is more coming) and then check it out and accept the pull request. And then I'll publish a new release to Maven Central soon.