Adposition typedef merging

Grammar Matrix
#1

When running the newer python3 version of the matrix on a choices file, there are some errors in lexicon.tdl. It attempts to merge adposition types and then comes up with tdl with invalid syntax as shown:

;;; Case-marking adpositions

in-marker := case-marking-adp-lex &
  [ STEM < "mia" & "mi-ng" & "mia" >,
    SYNSEM.LOCAL [ CONT [ HOOK [ ICONS-KEY.IARG1 #clause,
                                 CLAUSE-KEY #clause ],
                          ICONS.LIST < > ],
                   CAT.HEAD [ CASE in,
                              CASE-MARKED + ] ] ].

Where the STEM value is invalid.

I compared the python2 matrix version to the python3 version, and I found the difference that’s causing this problem:

    adp_type = TDLencode(abbr + '-marker')
    if adp_type in adp_type_names:
        adp_type = adp_type + '_2'
        while True:
            if adp_type not in adp_type_names:
                break
            adp_type = adp_type[:-1] + str(int(adp_type[-1])+1)
    adp_type_names.append(adp_type)
    
    typedef = \
        adp_type + ' := ' + super_type + ' & \
                [ STEM < "' + orth + '" > ].'
    lexicon.add(typedef)

Everything from the second line (if adp_type in adp_type_names) to the line just before setting the typedef (adp_type_names.append(adp_type)) has been removed in the new version. All this snippet does is create a new adposition type with an underscore+index to prevent it from passing the “if TDLmergeable” test that occurs later which tries merging the types. In the new version because the types have identical names it passes the mergeable test and then merges them into a type with invalid syntax.

So a potential solution would be to just put that snippet back in, but when that snippet is in the code, this is the resulting output:

in-marker := case-marking-adp-lex &
  [ STEM < "mia" >,
    SYNSEM.LOCAL [ CONT [ HOOK [ ICONS-KEY.IARG1 #clause,
                                 CLAUSE-KEY #clause ],
                          ICONS <!  !> ],
                   CAT.HEAD [ CASE in,
                              CASE-MARKED + ] ] ].

in-marker_2 := case-marking-adp-lex &
  [ STEM < "mi-ng" >,
    SYNSEM.LOCAL [ CONT [ HOOK [ ICONS-KEY.IARG1 #clause,
                                 CLAUSE-KEY #clause ],
                          ICONS <!  !> ],
                   CAT.HEAD [ CASE in,
                              CASE-MARKED + ] ] ].

in-marker_3 := case-marking-adp-lex &
  [ STEM < "mi-a" >,
    SYNSEM.LOCAL [ CONT [ HOOK [ ICONS-KEY.IARG1 #clause,
                                 CLAUSE-KEY #clause ],
                          ICONS <!  !> ],
                   CAT.HEAD [ CASE in,
                              CASE-MARKED + ] ] ].

It works, but it seems a little “improper” to me to have these three types as opposed to three lexical entries for the markers that inherit from one “in-marker” type. But, I have no idea how to tackle making an improvement like that. Is it worth bothering? Or should I put the old snippet back in that adds the “_2” etc and leave it at that for now?

Hierarchies in the matrix code
#2

What makes you think those are types and not lexical entries? They look like lex entries to me…

#3

Ah, I see. My mistake. So I’ll put that snippet back in to separate out the entries, thanks!

#4

PS how can I get access to push the branch I’m working on to the repo?

#5

Thanks for catching and fixing this!

#7

That would be on github now, wouldn’t it? I think we need to make you the right kind of contributor… @olzama or @goodmami: can you give me a quick tutorial on how I would do that?

#8

I am not sure :). I am listed only as a “member” in the matrix-dev project on GitHub myself. Emily, you are “maintainer” so I am assuming you can add a new member. I would imagine you might need to first add Liz as a member also to the delphin “organization” there (where I am also just a member and don’t seem to have such privileges).

But, @ecconrad, once that’s done, please do not just push to the trunk. Create a pull request so that everyone can take a look first.

Also, I once again wonder if this has something to do with the change to tdl.py that I had made (the same that I suspect may be causing the RELS.LIST issue). That change was related to if_mergeable(). So, this may need to be investigated further. If you put the snippet back in, do all regression tests pass?

#9

As a separate note, I wonder what puts in the "ICONS <! !> ". There should no longer be <! !> anywhere; this should be “ICONS.LIST < >”. Is it possible to track down where that came from?

#10

Another question: this version of the matrix that you are working with; is it completely fresh, checked out into a fresh location from the GitHub trunk? Not, say, copied over the old version or something? It is strange that this snippet is working put back in because I cannot find any mention of the variable adp_type_names in my version, for instance. So there seems to be a mixture of versions somewhere (which does happen).

#11

Sure. But first, note that there is no need to have write access to the repo in order to create pull requests. Developers can fork the repo, push a new branch, then create a pull request from it. A maintainer can then merge that pull request after review (as Olga requested). Unless the contributor actually needs more control (e.g., to close issue tickets, merge directly to trunk, or manage repo settings), this is the preferred workflow.

If you wish to give someone direct write access, you can either add them individually (collaborator) or as a matrix-devs team member. The latter requires that they are part of the delph-in organization on GitHub, which may grant them other privileges outside the Matrix repository. For a collaborator, go to the Matrix repository, click on the “Settings” tab:

settings

 
… then the “Manage access” link:

manage-access

 
… then the green “Invite teams or people” button:

invite

Then you can search for lizcconrad and create an invitation, and choose the “Write” role.

#12

The “ICONS <! !>” is definitely from an older version, I was showing it to show how the lexical entries look with the snippet that ensures they don’t get merged into one big lexical item with multiple STEM values.

The adp_type_names I believe is only in the old version but I put it back in to prevent the lexical entries from getting merged. It appeared to work but I don’t know how to check regression tests? How do I do that?

I believe I git pulled from the trunk, but I can always do it again to be sure.

#13

Thanks, @goodmami ! I think I can handle that. @ecconrad I’ve invited you to be a collaborator with write access, but please do create pull requests for someone to approve.

#14

Here’s some instructions; let me know if they are helpful enough (or not). I think you just need the part that says “Running regression tests”.

Then, instead of modifying the trunk, create a branch and then make the change in the branch. Check that all the tests pass; if they do, create a pull request on GitHub (let me know if you need any help with anything!)