#1443 - Make BratReader more forgiving

[alaindesilets]

This is a partial implementation of the improvements described in the following issue:

#1443

I need some help to move forward because I am hitting a snag.

If you run the tests on this branch as is, they work fine. But if you remove comments from lines 115-119 in Mapping.java, test1mapping fails, with the following exception:

java.lang.IllegalStateException: Duplicate key org.dkpro.core.io.brat.internal.mapping.SpanMapping@2098d37d

I have no idea how to fix that. Can someone more knowledgeable with the Mapping data structure take a look at this? Thx.

[ukp-svc-jenkins]

Can one of the admins verify this patch?

[reckart]

@alaindesilets I'm a bit confused seeing that there is another alaindesilets/Improvement/Make_BratReader_more_forgiving__Take2 branch in your repo - but there is no PR for it. What should I be looking at?

[alaindesilets]

Just hold off on everything. I was having problems with that branch (my skills with git in such complex scenarios are a bit shaky) and decide to restart from scratch. I have made some progress but am still struggling with how to merge two Mapping instances. If somebody could implement a static method in Mapping public static Mapping merge(Mapping m1, Mapping m2) I think I could handle the rest. Le dim. 22 déc. 2019 à 08:12, Richard Eckart de Castilho < [email protected]> a écrit :

…

@alaindesilets <https://github.com/alaindesilets> I'm a bit confused seeing that there is another alaindesilets/Improvement/Make_BratReader_more_forgiving__Take2 branch in your repo - but there is no PR for it. What should I be looking at? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1444?email_source=notifications&email_token=AAIMA4FTPXPTQHWSPLMNZGTQZ5RVJA5CNFSM4J3MUCV2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHPPREI#issuecomment-568260753>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIMA4FPKRF6HBYYPRR2TDLQZ5RVJANCNFSM4J3MUCVQ> .

[reckart]

Is that new work on the "take 2" branch then?

[alaindesilets]

Yes. I just issued a pull request for that one. Sorry about the confusion. I am still learning how to use git in forks and branches situations, and I got stuck in a corner with the first branch so I started from scratch on this Take2 branch.

…

On Sun, Dec 22, 2019 at 9:07 AM Richard Eckart de Castilho < ***@***.***> wrote: Is that new work on the "take 2" branch then? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1444?email_source=notifications&email_token=AAIMA4BBZVEK23VDOW4F2ODQZ5YC5A5CNFSM4J3MUCV2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHPQ5EA#issuecomment-568266384>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIMA4GW7UTWDVPWKTBAFNDQZ5YC5ANCNFSM4J3MUCVQ> .

[alaindesilets]

Another thing I should point out. In the Take2 branch, I implemented the changes so they will work for BratReader and made no attempt to make them consistent with the overall CollectionReader mechanics. For example, in order to get a list of the files that correspond to the various PARAMS reecived by the reader, I used my personal FileGlob library (which I am happy to contribute to the project if needed) as well as some new methods like sourceLocationIsSingleFile() instead of trying to figure out how to make the scan() method do what I want it to do. Since BratReader is the only CollectionReader I ever used I didn't feel like investing the time to improve the more general CollectionReader functionality. But maybe someone else can reproduce these improvements for the more generic case. On Sun, Dec 22, 2019 at 9:23 AM Alain Désilets <[email protected]> wrote:

…

Yes. I just issued a pull request for that one. Sorry about the confusion. I am still learning how to use git in forks and branches situations, and I got stuck in a corner with the first branch so I started from scratch on this Take2 branch. On Sun, Dec 22, 2019 at 9:07 AM Richard Eckart de Castilho < ***@***.***> wrote: > Is that new work on the "take 2" branch then? > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#1444?email_source=notifications&email_token=AAIMA4BBZVEK23VDOW4F2ODQZ5YC5A5CNFSM4J3MUCV2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHPQ5EA#issuecomment-568266384>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AAIMA4GW7UTWDVPWKTBAFNDQZ5YC5ANCNFSM4J3MUCVQ> > . >

[alaindesilets]

Also, I did not use testOneWay() for my tests, and instead wrote a new method testReadWrite(). The problem with testOneWay() is that it does a lot of checking of the reader/writer's params in order to "fix them" and prevent the kind of common error condition that I trying to deal with. For example, if you feed a PARAM_SOURCE_LOCATION that does not have a *.ann at the end, testOneWay() adds it automatically, which means I can never write a failing test for that condition using testOneWay(). On Sun, Dec 22, 2019 at 9:29 AM Alain Désilets <[email protected]> wrote:

…

Another thing I should point out. In the Take2 branch, I implemented the changes so they will work for BratReader and made no attempt to make them consistent with the overall CollectionReader mechanics. For example, in order to get a list of the files that correspond to the various PARAMS reecived by the reader, I used my personal FileGlob library (which I am happy to contribute to the project if needed) as well as some new methods like sourceLocationIsSingleFile() instead of trying to figure out how to make the scan() method do what I want it to do. Since BratReader is the only CollectionReader I ever used I didn't feel like investing the time to improve the more general CollectionReader functionality. But maybe someone else can reproduce these improvements for the more generic case. On Sun, Dec 22, 2019 at 9:23 AM Alain Désilets ***@***.***> wrote: > Yes. I just issued a pull request for that one. > > Sorry about the confusion. I am still learning how to use git in forks > and branches situations, and I got stuck in a corner with the first branch > so I started from scratch on this Take2 branch. > > On Sun, Dec 22, 2019 at 9:07 AM Richard Eckart de Castilho < > ***@***.***> wrote: > >> Is that new work on the "take 2" branch then? >> >> — >> You are receiving this because you were mentioned. >> Reply to this email directly, view it on GitHub >> <#1444?email_source=notifications&email_token=AAIMA4BBZVEK23VDOW4F2ODQZ5YC5A5CNFSM4J3MUCV2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHPQ5EA#issuecomment-568266384>, >> or unsubscribe >> <https://github.com/notifications/unsubscribe-auth/AAIMA4GW7UTWDVPWKTBAFNDQZ5YC5ANCNFSM4J3MUCVQ> >> . >> >

[reckart]

Superseded by #1448