TS-9 final change - compare with branch name TS-9 #8
Conversation
johnholt
left a comment
johnholt
left a comment
There was a problem hiding this comment.
There are many files that look like your tools. Not named for other people to use, and not made general. Also, the ingest of the Moby data is incomplete.
My most significant problem though is it looks like you forgot to rebase and so dropped the last merge.
| //Default implementation. Provides minimal functionality. | ||
| IMPORT Std.Uni; | ||
| //from me this file give defult values to varibles and interface in Ikeywording file | ||
| IMPORT Std.Uni; //to use lower case or upper case (from me) |
There was a problem hiding this comment.
I don't verbally comment on comments, but you should drop the "form me" notes.
| IMPORT TextSearch.Common.Layouts; | ||
| TermString := Types.TermString; | ||
| EquivTerm := Layouts.EquivTerm; | ||
| EquivTerm := Layouts.EquivTerm; //store all record |
| EXPORT UNSIGNED2 DataVersion := 0; // placeholder for data version to build | ||
| EXPORT UNSIGNED1 Levels := 5; | ||
| EXPORT STRING UseInstance(UNSIGNED indx) := IF(indx=0, Instance, AliasInstances[indx]); | ||
| EXPORT UNSIGNED2 Naming := 1; |
There was a problem hiding this comment.
You are dropping out the work of a prior merge. Why? Did you forget to rebase?
| FileName_Info := Common.FileName_Info; | ||
|
|
||
| EXPORT FileNames(FileName_Info info, UNSIGNED Alias=0) := MODULE | ||
| EXPORT FileNames(FileName_Info info) := MODULE //to set name of doc |
There was a problem hiding this comment.
You are dropping out prior work. Why?
| SHARED Name(STRING suffix, UNSIGNED lvl) := info.Prefix + DocSearchPrefix | ||
| + INTFORMAT(lvl, 2, 1) + '::' | ||
| + info.UseInstance(Alias) + '::' + suffix; | ||
| + info.Instance + '::' + suffix; |
There was a problem hiding this comment.
You are dropping out prior work. Why?
| PATTERN XMLElement := U'<' XMLName BEFORE ContainerEnd; | ||
| PATTERN XMLEmpty := U'<' XMLName BEFORE EmptyEnd; | ||
| PATTERN expr2 :=PATTERN(U'[a-zA-Z]+[.][a-zA-Z]+[.][a-zA-Z]*[.]*[a-zA-Z]*'); | ||
| PATTERN expr3 :=PATTERN(U'[a-zA-Z]+[.][a-zA-Z]+'); |
There was a problem hiding this comment.
It would be more helpful if the names were meaningful.
| PATTERN XMLElement := U'<' XMLName BEFORE ContainerEnd; | ||
| PATTERN XMLEmpty := U'<' XMLName BEFORE EmptyEnd; | ||
| PATTERN expr2 :=PATTERN(U'[a-zA-Z]+[.][a-zA-Z]+[.][a-zA-Z]*[.]*[a-zA-Z]*'); | ||
| PATTERN expr3 :=PATTERN(U'[a-zA-Z]+[.][a-zA-Z]+'); |
There was a problem hiding this comment.
These rules look like they only work in the US and UK.
Also, I don't see what you are trying to accomplish with the "expr3" pattern.
| MATCHED(AnyPair) => 1, | ||
| 0); | ||
| MATCHED(expr2) => MATCHLENGTH(expr2)- STD.Str.FindCount((STRING)MATCHTEXT(expr2), '.'),//new addition//track dot here | ||
| MATCHED(expr3) => MATCHLENGTH(expr3)- STD.Str.FindCount((STRING)MATCHTEXT(expr3), '.'), |
There was a problem hiding this comment.
A function would be better for readability instead of an inline definition. Again, I suspect that you forgot to fix your environment to force tabs to spaces.
| enumDocs := Inverted.EnumeratedDocs(info, ds1); | ||
| p1 := Inverted.ParsedText(enumDocs); | ||
| rawPostings := Inverted.RawPostings(enumDocs); | ||
| OUTPUT(CHOOSEN(p1,30)); |
| @@ -0,0 +1,54 @@ | |||
| //EXPORT check := 'todo'; | |||
There was a problem hiding this comment.
Attribute is poorly named. It would also be better if the attribute returned an action instead of just being runnable. Move the EXPORT to the end, make the output actions attributes, and then define the export to be an action composed of the attribute names of the previous actions.
|
Thanks @johnholt for reviewing these and offering comments. Farah was having issues committing changes and possibly missed a step when trying to re-sync. She is going to try starting from a fresh fork/clone and then adding her new files only- then doing a manual merge. Also could you give a little more detail on the incomplete Moby ingest? Is there a file in particular that is missing something? Thanks!! |
|
In the ideal case, a Moby module would include the file services calls to spray the data set or datasets. At a minimum, the Moby module should include directions (in comments) on what files to download; how to spray those files; and the ECL to convert the raw text files into the form used for the equivalencing process. |
|
Also, the pull request name could be better. There are examples in the Wiki for HPCC-Platform, |
|
@RogerDev What should be happening to this PR? |
5 participants
No description provided.