i_1195 Handle escapes in institution names#1196
Conversation
Fix TabSeparatedValueParser to allow backslash escaping of characters in fields. CI: Allow the timelimit in the CLICS section to be a Double (SEERC contest did this) CI: Add better exception handling if invalid institution information is returned
Fixed NPE added by adding Double processing.
| Double clicsTimeout = fetchDoubleValue(limitsContent, TIMEOUT_KEY); | ||
| if (clicsTimeout != null) { | ||
| problem.setTimeOutInSeconds(clicsTimeout); | ||
| problem.setTimeOutInSeconds(clicsTimeout.intValue()); |
There was a problem hiding this comment.
Why is it 'intValue()' instead of 'doubleValue()'?
There was a problem hiding this comment.
This is because we found that some contests (especially the Europe ones), specify the problem timeout as a floating point number in the problem.yaml, eg. timeout=2.0. They are using the latest specification of the problem package format, which ICPC does not use yet in their CCS's. This causes an exception if you try to fetch it from the map as an integer, so we now read it as a double, then convert it to an integer (which is what the old specification wants).
SamanwaySadhu
left a comment
There was a problem hiding this comment.
The code changes look good. However, this PR does not have the steps of what to test. Please let me know if I am only supposed to review the code or recreation steps will be added later.
You can try to follow the procedure outlined in the Issue #1196 - it mentions the SEERC2025 contest, which I invited you to on git. It's in the contest/seerc2025. Use the "pc2new" branch (git checkout pc2). |
The above reply references "Issue #1196". That's actually the PR number (of THIS PR). I think it was intended to reference "Issue #1195". |
clevengr
left a comment
There was a problem hiding this comment.
I have reviewed the code, and with the exception of a trivial comment about the use of a deprecated constructor in one file, the code looks fine to me. However, I cannot figure out how to test it. Samanway asked this same question, and the answer (as I interpret it) seemed to be "use the contest in https://github.com/icpcsysops/SEERC2025/tree/main/contests/seerc2025". However, I've looked extensively through that contest (including in the ../../shared folder) and I can't find any institutions.tsv file. Since this PR is about handling quoted strings in institutions.tsv, I expected there would be such a file in that contest. What are we missing? Is the expectation that we would create our own institutions.tsv file for testing? (I can certainly do that, but my impression is that there is supposedly already sufficient data available to make testing supported as-is...)
Instructions above say to use the "pc2new" branch of the SEERC contest repo. |
I do not see a "pc2new" branch anywhere under the SEERC2025 repo. Can you explain where to find that? |
I had a link in the message above: https://github.com/icpcsysops/SEERC2025/tree/pc2new/contests Or, from command line: |
I still cannot find the config containing Next, I cd'd into the cloned repo folder and then followed your sequence of instructions: Note that at this point my result differs from yours: there is no Note that this is also different: your instructions indicate it should say Again, different output here, but still I'm thinking "maybe this is ok..." So I continued: So I'm not getting the I tried running the Any ideas why I'm not getting the repo cloned properly? |
Sure, because you don't have it on your local drive yet (you didn't check it out), but the REMOTE one is there which is where it will get it from when you check it as, as you did below. So far everything is as expected.
Perfect.
Yeah, because this was done on my home computer, not my laptop and my home computer is probably slightly behind the branch on git. That's fine. You would get the latest one which is what you want.
Perfect
Sounds like something is overriding your symlinks flag; clearly that's the problem. You should be able to go directly to where the symlink points to see if the institutions file is in fact there (of course, it will be).
Well, that just tells you what the GLOBAL settings are, not what was being used. I think you need just:
I would also try looking at the folder using explorer, and see if it sees the symlinks in the config folder - there should be a couple - teams2.tsv, groups.tsv also are symlinks to the shared/pc2config folder files. If they do not show up as symlinks then your git is not cloning right for some reason. (I thought you had this issue with another repo as well?) I might also try the git from a command prompt instead of PS - again, I don't know what PS may be doing. |
Ok, this is REALLY weird. I tried recloning, using However, when I actually try to OPEN one of the symlinked files, for example by using the command |
|
Ok, that's strange. Explorer should show what I have above: It should say SYMLINK and not size 0. It DOES say the total size of the directory listing is 0, but that's to be expected... |
clevengr
left a comment
There was a problem hiding this comment.
I attempted to load the "SEERC2025" contest into a build created from Develop; it threw an "Array index out of bounds" exception (as expected, as reported in the initial Issue). I then loaded the same contest into the PR (fixed) distribution; it loaded without error and when I looked at a PC2 "Accounts" report I see the following for team 1119974 (the team which was generating the Exception):
Institution name '"Gheorghe Asachi" Technical University of Ia?i' Institution code 'INST-10088' Institution short name 'TUIASI'
I do note that there is a non-ASCII character in the name ("Ia?i") which is not being handled properly, but that's outside the scope of the issue which this PR fixes. I approve the PR.
SamanwaySadhu
left a comment
There was a problem hiding this comment.
Works as expected!
PR Change request: Use Double.valueOf() instead of new Double().
Description of what the PR does
Fix
TabSeparatedValueParserto allow backslash escaping of characters in fields.CI: Allow the
timelimitproperty in the CLICS section to be aDouble(SEERC contest did this)CI: Add better exception handling if invalid institution information is returned
Issue which the PR addresses
Fixes #1195
Environment in which the PR was developed (OS,IDE, Java version, etc.)
Precise steps for testing the PR (i.e., how to demonstrate that it works correctly)
TBD