gf-core icon indicating copy to clipboard operation
gf-core copied to clipboard

"Majestic" PGF runtime

Open johnjcamilleri opened this issue 3 years ago • 91 comments

@krangelov has mentioned at the summer school his ideas on completely reimagining the PGF format as a database. @anka-213 has expressed his interest in contributing, and I (@johnjcamilleri) also think I could be of use here. I'm creating this issue to be the home of that discussion.

johnjcamilleri avatar Aug 09 '21 09:08 johnjcamilleri

For the current status look at branch 'majestic':

https://github.com/GrammaticalFramework/gf-core/tree/majestic/src/runtime/c

The subfolder 'doc' contains documentation for the decisions that I made so far. My ambition is that the doc folder would develop into a living book documenting the internals. Hopefully, it will help new developers to jump in later.

What works so far:

  • the C runtime can read the abstract syntax of a .pgf and build an .ngf file from it. The .ngf is the actual database. Some of the abstract syntax functions in the API are implemented as well.

  • the old Haskell runtime is gone, the folder runtime/haskell contains the new Haskell binding. There are two top-level modules PGF and PGF2. PGF tries to emulate the API of the old Haskell runtime, while PGF2 should be compatible with the API of the C runtime. Everything that already works in the C library has also a binding in the Haskell runtime. There is also a testsuite that I use to test the Haskell binding together with the actual runtime.

  • most of the code in the Python binding is commented out. It compiles, but the only thing that you can do there is to load a grammar. I only needed that in order to test that it is possible to load the runtime dynamically.

  • the code for the compiler comes from the branch 'c-runtime'. In that branch the compiler runs on top of the C runtime instead of the Haskell one. In the new branch the compiler doesn't compile yet since not everything in the runtime is implemented yet. Part of the compiler must be updated anyway, since if we want dynamically changing grammars, the grammar building API would have to change anyway.

Current milestones:

  • Finish the rest of the readonly API for the abstract syntax
  • Add API that can add/remove abstract functions and categories
  • Implement proper transactions
  • Modify the compiler to make it possible to compile grammars with only abstract syntax
  • Modify the compiler to produce an extended version PMCFG which would not blow up in size so easily
  • Load the new PMCFG in the runtime
  • Continue with the different API functions for the concrete syntax. Perhaps linearization should be the first to start with.

If you want to join the effort then we should schedule a meeting and decide how to proceed.

On Mon, 9 Aug 2021 at 11:05, John J. Camilleri @.***> wrote:

@krangelov https://github.com/krangelov has mentioned at the summer school his ideas on completely reimagining the PGF format as a database. @anka-213 https://github.com/anka-213 has expressed his interest in contributing, and I @.*** https://github.com/johnjcamilleri) also think I could be of use here. I'm creating this issue to be the home of that discussion.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GrammaticalFramework/gf-core/issues/130, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEYFSZHT3E245XBNRLP5A2TT36K4HANCNFSM5BZSYMKQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

krangelov avatar Aug 10 '21 09:08 krangelov

Wow, this seems a lot more advanced than you let on! I will take some time to better understand how it works.

johnjcamilleri avatar Aug 10 '21 10:08 johnjcamilleri

Hi @krangelov, pretty cool work! I remember hearing something about using SQLite for this work, which would give you transactions for free and also a platform-independent format (I'm assuming it would take the role of the .ngf files). Have you decided against it? I'm just curious and would love to contribute (I don't think I can, but I'll follow the discussion and if it turns out I can I'll say something!)

odanoburu avatar Aug 10 '21 11:08 odanoburu

I tried using SQLite but it quickly becomes very cumbersome to work in this way. It means that every time when I must follow a pointer from one structure to another, I must open/close a cursor and do a few more function calls until I get what I want. In addition everything must be serialized/deserialized to a platform independent format.

I am not planning to use it anymore, but here is an interesting spin off project from the experiment:

https://github.com/krangelov/daison

It is a NoSQL database where you can use list comprehensions instead of SQL. Hopefully, something similar will be available for storing auxiliary information in grammars.

On Tue, 10 Aug 2021 at 13:23, bc² @.***> wrote:

Hi @krangelov https://github.com/krangelov, pretty cool work! I remember hearing something about using SQLite for this work, which would give you transactions for free and also a platform-independent format (I'm assuming it would take the role of the .ngf files). Have you decided against it? I'm just curious and would love to contribute (I don't think I can, but I'll follow the discussion and if it turns out I can I'll say something!)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GrammaticalFramework/gf-core/issues/130#issuecomment-895948205, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEYFSZHVMCVEN7KNYJIHQRDT4ED2JANCNFSM5BZSYMKQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

krangelov avatar Aug 10 '21 11:08 krangelov

Yes, it looks majestic to me, and the documents read well! Although the details about memory addresses are tricky.

I agree that linearization would be the first thing to make available, so that we can see if this is all we need of a linearization format in a large-scale setting.

Aarne


From: John J. Camilleri @.***> Sent: Tuesday, August 10, 2021 12:10:35 PM To: GrammaticalFramework/gf-core Cc: Subscribed Subject: Re: [GrammaticalFramework/gf-core] Next-generation PGF (#130)

Wow, this seems a lot more advanced than you let on! I will take some time to better understand how it works.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/GrammaticalFramework/gf-core/issues/130#issuecomment-895903811, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAWBQXNCW6VC573ASGXGO53T4D3JVANCNFSM5BZSYMKQ.

aarneranta avatar Aug 10 '21 14:08 aarneranta

@krangelov I see, thanks for the explanation! and daison looks interesting indeed!

odanoburu avatar Aug 10 '21 15:08 odanoburu

I figure the best way I can start contributing to this effort is to be your first user! Which probably means I will have lots of annoying setup questions. To start with, I tried to build the runtime using the instructions from the old C runtime, which failed. I set up a CI workflow here which documents what I tried and also contains the current error I get. I'm happy to contribute to other kinds of documentation (as library docs and how-to guides) as I learn more.

Going forward, we should add whatever testsuite you have (mentioned earlier) to this workflow.

johnjcamilleri avatar Aug 12 '21 08:08 johnjcamilleri

To be explicit, I am trying:

autoreconf -i
./configure
make

and getting:

make[1]: Entering directory '/home/runner/work/gf-core/gf-core/src/runtime/c'
  CXX      pgf/db.lo
  CXX      pgf/text.lo
  CXX      pgf/pgf.lo
  CXX      pgf/reader.lo
make[1]: *** No rule to make target 'pgf/expr.cxx', needed by 'pgf/expr.lo'.  Stop.
make[1]: Leaving directory '/home/runner/work/gf-core/gf-core/src/runtime/c'

What am I doing wrong?

johnjcamilleri avatar Aug 12 '21 11:08 johnjcamilleri

I added expr.cxx to the repo, so it should compile now.

To run the testsuite, you should run:

cabal test

in runtime/haskell.

On Thu, 12 Aug 2021 at 13:07, John J. Camilleri @.***> wrote:

To be explicit, I am trying:

autoreconf -i ./configure make

and getting:

make[1]: Entering directory '/home/runner/work/gf-core/gf-core/src/runtime/c' CXX pgf/db.lo CXX pgf/text.lo CXX pgf/pgf.lo CXX pgf/reader.lo make[1]: *** No rule to make target 'pgf/expr.cxx', needed by 'pgf/expr.lo'. Stop. make[1]: Leaving directory '/home/runner/work/gf-core/gf-core/src/runtime/c'

What am I doing wrong?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GrammaticalFramework/gf-core/issues/130#issuecomment-897548450, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEYFSZGP7DCQCBVLQJY4LGLT4OTQ5ANCNFSM5BZSYMKQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

krangelov avatar Aug 13 '21 06:08 krangelov

Thanks Krasimir, the CI workflow now builds everything and runs the tests successfully. I will keep poking around to understand things better. Probably the next thing I can work on is adding more tests.

johnjcamilleri avatar Aug 13 '21 09:08 johnjcamilleri

most of the code in the Python binding is commented out. It compiles, but the only thing that you can do there is to load a grammar. I only needed that in order to test that it is possible to load the runtime dynamically.

As far as I can see, nothing is commented out in src/runtime/python/pypgf.c?

I added a simple test to load the basic.pgf file from Python:

import pgf
import sys

sys.stdout.write("loading...")
sys.stdout.flush();
gr = pgf.readPGF("../haskell/tests/basic.pgf")
sys.stdout.write("\n")

but I get a segmentation fault. The CI output is here, and I get the same locally.

johnjcamilleri avatar Aug 24 '21 07:08 johnjcamilleri

Actually commented out means that it is disabled with:

#if 0 #endif

You can see that on line 42. Comments in C/C++ are not recursive and are not as nice as in Haskell.

Otherwise the Python binding used to work until I broke it while developing the Haskell binding. You can find the comment:

/TO BE FIXED/

on line 3516.

I am working on the coming Python course now, but I hope to return to the runtime soon.

On Tue, 24 Aug 2021 at 09:14, John J. Camilleri @.***> wrote:

most of the code in the Python binding is commented out. It compiles, but the only thing that you can do there is to load a grammar. I only needed that in order to test that it is possible to load the runtime dynamically.

As far as I can see, nothing is commented out in src/runtime/python/pypgf.c?

I added a simple test https://github.com/GrammaticalFramework/gf-core/blob/majestic/src/runtime/python/test.py to load the basic.pgf file from Python:

import pgfimport sys sys.stdout.write("loading...")sys.stdout.flush();gr = pgf.readPGF("../haskell/tests/basic.pgf")sys.stdout.write("\n")

but I get a segmentation fault. The CI output is here https://github.com/GrammaticalFramework/gf-core/runs/3408397541?check_suite_focus=true, and I get the same locally.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GrammaticalFramework/gf-core/issues/130#issuecomment-904384489, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEYFSZFRZ2FZ54U4FWVSWDDT6NBDXANCNFSM5BZSYMKQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

krangelov avatar Aug 24 '21 07:08 krangelov

Ah, thanks for the clarification!

johnjcamilleri avatar Aug 24 '21 08:08 johnjcamilleri

Hi Krasimir, I'm not experienced in C++, but I think this work is so important to the future of GF that it is worth me taking time to learn C++ itself as well as the internal details of how this new runtime works. I might be slow initially, but perhaps you can think of some simpler tasks to give me now, to help me get up to speed?

johnjcamilleri avatar Aug 26 '21 06:08 johnjcamilleri

I would appreciate it if you could start updating the Python bindings. Before that give me about a week to finish and push some local changes that I have. After that we could have a chat about what needs to be done.

On Thu, 26 Aug 2021 at 08:58, John J. Camilleri @.***> wrote:

Hi Krasimir, I'm not experienced in C++, but I think this work is so important to the future of GF that it is worth me taking time to learn C++ itself as well as the internal details of how this new runtime works. I might be slow initially, but perhaps you can think of some simpler tasks to give me now, to help me get up to speed?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GrammaticalFramework/gf-core/issues/130#issuecomment-906146145, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEYFSZHWW5IJZ3DTV3NWYEDT6XQYTANCNFSM5BZSYMKQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

krangelov avatar Aug 26 '21 09:08 krangelov

The Python binding works again. I also reached the milestone where all essential functions for the abstract syntax work.

There are a lot of low hanging fruits in the Python binding. These functions should be easy to add:

pgf_readNGF -- analogous to readNGF in the Haskell runtime, it can be built by modifying pgf_readPGF pgf_bootNGF -- analogous to bootNGF in the Haskell runtime, it can be built by modifying pgf_readPGF

These are also easy to restore from the old code:

PGF_getAbstractName PGF_getCategories PGF_getFunctions PGF_functionsByCat

The next bigger chunk is:

PGF_functionType -- get the type of a function PGF_getStartCat -- get the type for the start category Expr_repr -- to show an expression as a string pgf_readExpr -- read an expression from a string Type_repr -- to show a type as a string pgf_readType -- read an expression from a string

The last six functions require that the marshalling of abstract expressions and types is implemented for Python as well. I started writing the documentation about how this works.

On Thu, 26 Aug 2021 at 11:47, Krasimir Angelov @.***> wrote:

I would appreciate it if you could start updating the Python bindings. Before that give me about a week to finish and push some local changes that I have. After that we could have a chat about what needs to be done.

On Thu, 26 Aug 2021 at 08:58, John J. Camilleri @.***> wrote:

Hi Krasimir, I'm not experienced in C++, but I think this work is so important to the future of GF that it is worth me taking time to learn C++ itself as well as the internal details of how this new runtime works. I might be slow initially, but perhaps you can think of some simpler tasks to give me now, to help me get up to speed?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GrammaticalFramework/gf-core/issues/130#issuecomment-906146145, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEYFSZHWW5IJZ3DTV3NWYEDT6XQYTANCNFSM5BZSYMKQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

krangelov avatar Aug 27 '21 10:08 krangelov

Great, I will start working on getting these implemented.

johnjcamilleri avatar Aug 27 '21 11:08 johnjcamilleri

Checklist for Python bindings:

Reading from file

  • [x] pgf_readNGF
  • [x] pgf_bootNGF

Restore from old code

  • [x] PGF_getAbstractName
  • [x] PGF_getCategories
  • [x] PGF_getFunctions
  • [x] PGF_functionsByCat

Require marshalling of abstract expressions

  • [x] PGF_functionType
  • [x] PGF_getStartCat
  • [x] Expr_repr
  • [x] pgf_readExpr
  • [x] Type_repr
  • [x] pgf_readType

johnjcamilleri avatar Aug 30 '21 08:08 johnjcamilleri

So I have managed to implement the simpler functions for getting categories/functions by restoring old code, but one thing I can't figure out is how the error handing is supposed to work. Namely, the GuExn* err parameter seems to have disappeared from functions such as pgf_iter_categories.

For now the old error-handling code is commented out like so:

static PyObject*
PGF_getCategories(PGFObject *self, void *closure)
{
    PyObject* categories = PyList_New(0);
    if (categories == NULL)
        return NULL;

    // GuPool* tmp_pool = gu_local_pool();
    //
    // Create an exception frame that catches all errors.
    // GuExn* err = gu_new_exn(tmp_pool);

    PyPGFClosure clo = { { pgf_collect_cats }, self, categories };
    pgf_iter_categories(self->pgf, &clo.fn);

    // if (!gu_ok(err)) {
    //     Py_DECREF(categories);
    //     gu_pool_free(tmp_pool);
    //     return NULL;
    // }
    //
    // gu_pool_free(tmp_pool);
    return categories;
}

Any pointers here?

johnjcamilleri avatar Aug 30 '21 21:08 johnjcamilleri

pgf_iter_categories & pgf_iter_functions doesn't really generate any errors. That is why the parameter has disappeared. Other functions that may fail now take PgfExn as a parameter. See pgf_read_pgf for example.

On Mon, 30 Aug 2021 at 23:29, John J. Camilleri @.***> wrote:

So I have managed to implement the simpler functions for getting categories/functions by restoring old code, but one thing I can't figure out is how the error handing is supposed to work. Namely, the GuExn* err parameter seems to have disappeared from functions such as pgf_iter_categories.

For now the old error-handling code is commented out like so:

static PyObject*PGF_getCategories(PGFObject *self, void closure) { PyObject categories = PyList_New(0); if (categories == NULL) return NULL;

// GuPool* tmp_pool = gu_local_pool();
//
// Create an exception frame that catches all errors.
// GuExn* err = gu_new_exn(tmp_pool);

PyPGFClosure clo = { { pgf_collect_cats }, self, categories };
pgf_iter_categories(self->pgf, &clo.fn);

// if (!gu_ok(err)) {
//     Py_DECREF(categories);
//     gu_pool_free(tmp_pool);
//     return NULL;
// }
//
// gu_pool_free(tmp_pool);
return categories;

}

Any pointers here?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GrammaticalFramework/gf-core/issues/130#issuecomment-908712830, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEYFSZEZRYLD6LWDGRLBGXLT7PZ5JANCNFSM5BZSYMKQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

krangelov avatar Aug 31 '21 06:08 krangelov

Ok, what about the cases below? It seems possible that PyString_FromString and PyList_Append can fail. Do you think it's safe to ignore those too?

static void
pgf_collect_funs(PgfItor* fn, PgfText* key, void* value)
{
    PgfText* name = key;
    PyPGFClosure* clo = (PyPGFClosure*) fn;

    PyObject* py_name = NULL;

    py_name = PyString_FromString(name->text);
    if (py_name == NULL) {
        // gu_raise(err, PgfExn);
        goto end;
    }

    if (PyList_Append((PyObject*) clo->collection, py_name) != 0) {
        // gu_raise(err, PgfExn);
        goto end;
    }

end:
    Py_XDECREF(py_name);
}

johnjcamilleri avatar Aug 31 '21 06:08 johnjcamilleri

These error should be handled. When the Python API reports an error it also raises a Python exception. We just have to detect that and stop the iteration. I changed the iterator API to receive PgfExn like in the old runtime. I also updated the Python code to compile but I didn't fix the error handling. You just have to set

err.type = PGF_EXN_OTHER_ERROR;

when a problem on the Python side is encountered. This will stop the iterator. At the end you should also release the reference to the partly constructed list.

Another issue is that you should not use PyString_FromString anymore. This function expects that the input string is null terminated, while the new API allows input strings that contain the zero character \0. Instead you can use:

PyString_FromStringAndSize(name->text, name->size);

This would be also more efficient since Python would not need to compute the length itself.

On Tue, 31 Aug 2021 at 08:33, John J. Camilleri @.***> wrote:

Ok, what about the cases below? It seems possible that PyString_FromString and PyList_Append can fail. Do you think it's safe to ignore those too?

static voidpgf_collect_funs(PgfItor* fn, PgfText* key, void* value) { PgfText* name = key; PyPGFClosure* clo = (PyPGFClosure*) fn;

PyObject* py_name = NULL;

py_name = PyString_FromString(name->text);
if (py_name == NULL) {
    // gu_raise(err, PgfExn);
    goto end;
}

if (PyList_Append((PyObject*) clo->collection, py_name) != 0) {
    // gu_raise(err, PgfExn);
    goto end;
}

end: Py_XDECREF(py_name); }

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GrammaticalFramework/gf-core/issues/130#issuecomment-908941103, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEYFSZATOLJXOLBBBW44QC3T7RZT3ANCNFSM5BZSYMKQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

krangelov avatar Aug 31 '21 07:08 krangelov

Thanks Krasimir. On the subject of strings, does this look right to you?:

static PyObject*
PGF_functionsByCat(PGFObject* self, PyObject *args)
{
    const char* c;
    if (!PyArg_ParseTuple(args, "s", &c))
        return NULL;

    const size_t s = strlen(c);

    PgfText* catname = (PgfText*) alloca(sizeof(PgfText)+s);
    strcpy(catname->text, c);
    catname->size = s;

    ...

I need to read the parameter from args into a PgfText (previously PgfCId). But PyArg_ParseTuple wants to put it into a char*, so I have to do this allocation + copy. I'm guessing strlen also works on null-terminated strings, but maybe here this is the correct solution since the string in the arguments not coming from the runtime?

johnjcamilleri avatar Aug 31 '21 08:08 johnjcamilleri

I changed functionsByCat to use s# instead. In this way you get both the string and its length. This will work even if the function is called with a string containing the zero character.

Also instead of strcpy I used:

memcpy(catname->text, s, size+1);

Python always adds one zero character at the end and I copy that one as well. The idea is that although PgfText may contain strings containing '\0', most of the time that character is not used. If I append it at the end of the string then when I debug the runtime through gdb it correctly shows me the string as long as it doesn't contain '\0'.

Best Regards, Krasimir

On Tue, 31 Aug 2021 at 10:21, John J. Camilleri @.***> wrote:

Thanks Krasimir. On the subject of strings, does this look right to you?:

static PyObjectPGF_functionsByCat(PGFObject self, PyObject args) { const char c; if (!PyArg_ParseTuple(args, "s", &c)) return NULL;

const size_t s = strlen(c);

PgfText* catname = (PgfText*) alloca(sizeof(PgfText)+s);
strcpy(catname->text, c);
catname->size = s;

...

I need to read the parameter from args into a PgfText (previously PgfCId). But PyArg_ParseTuple wants to put it into a char*, so I have to do this allocation + copy. I'm guessing strlen also works on null-terminated strings, but maybe here this is the correct solution since the string in the arguments not coming from the runtime?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GrammaticalFramework/gf-core/issues/130#issuecomment-909011524, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEYFSZHJOLXCC6AZDHZX7CTT7SGH7ANCNFSM5BZSYMKQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

krangelov avatar Aug 31 '21 16:08 krangelov

I've starting working on the marshalling in the Python bindings (see 3ecb93775). I think it's starting to make sense, but I'd like to run a few things by you.

  • What should TypeObject contain (see expr.h)? Currently I have just a PgfText* in there for the type, but it needs more structure for the hypotheses etc. But am I correct in that we do not need a PgfType* in there anymore?
  • Next, when building a type object to return in the unmarshaller (see dtyp in marshaller.c), I am allocating a TypeObject and then returning it by casting it to a PgfType. Is that right?
  • Then, when using something which is created by the unmarshaller (see pgf_readType in pypgf.c), I just cast it back to a TypeObject* again. I think this aligns with what you wrote in the documentation, but would appreciate if you you confirm this.

johnjcamilleri avatar Sep 03 '21 12:09 johnjcamilleri

  • What should TypeObject contain (see expr.h)? Currently I have just a PgfText* in there for the type, but it needs more structure for the hypotheses etc. But am I correct in that we do not need a PgfType* in there anymore?

In TypeObject, I would put something like:

typedef struct { PyObject_HEAD PyList* hypos; PyString cat; PyList exprs; } TypeObject;

In this way all fields use native Python types. The list hypos should contain tuples with binding type, variable and the type of the variable. The list exprs should contain objects of type ExprObject.

  • Next, when building a type object to return in the unmarshaller (see dtyp in marshaller.c), I am allocating a TypeObject and then returning it by casting it to a PgfType. Is that right?

Yes. That is right

  • Then, when using something which is created by the unmarshaller (see pgf_readType in pypgf.c), I just cast it back to a TypeObject* again. I think this aligns with what you wrote in the documentation, but would appreciate if you you confirm this.

Exactly!

krangelov avatar Sep 05 '21 09:09 krangelov

In TypeObject, I would put something like:

typedef struct {
    PyObject_HEAD
    PyList* hypos;
    PyString *cat;
    PyList* exprs;
} TypeObject;

In this way all fields use native Python types. The list hypos should contain tuples with binding type, variable and the type of the variable. The list exprs should contain objects of type ExprObject.

Understood! What about the member PyObject* master, is this still relevant? I only half understood what its purpose is/was.

johnjcamilleri avatar Sep 06 '21 06:09 johnjcamilleri

It is not relevant anymore since everything lives in the Python heap.

On Mon, 6 Sept 2021 at 08:34, John J. Camilleri @.***> wrote:

In TypeObject, I would put something like:

typedef struct { PyObject_HEAD PyList* hypos; PyString cat; PyList exprs; } TypeObject;

In this way all fields use native Python types. The list hypos should contain tuples with binding type, variable and the type of the variable. The list exprs should contain objects of type ExprObject.

Understood! What about the member PyObject* master, is this still relevant? I only half understood what its purpose is/was.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GrammaticalFramework/gf-core/issues/130#issuecomment-913381608, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEYFSZCRH5QMYI7DST5T6O3UAROGHANCNFSM5BZSYMKQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

krangelov avatar Sep 06 '21 08:09 krangelov

The marshalling and types and expressions in the Python bindings is almost done. One thing in particular which I don't understand is the treatment of single quotes in identifier names.

In the Haskell tests you have:

TestCase (assertEqual "unicode names 2" (Just "ab") (fmap (showExpr []) (readExpr "'ab'")))

Which indicates that the outer quotes around the identifier are dropped when not needed. In Python I have this test as:

def test_readExpr_efun_str_unicode_2():
    assert pgf.showExpr([], pgf.readExpr("'аb'")) == "аb"

However this fails, but passes with ... == "'ab'". So in Python the outer quotes aren't being dropped. I would expect this behaviour to come from the runtime; we are both using pgf_print_expr and pgf_read_expr rather directly, and I don't see that you do anything in particular for quotes in the Haskell bindings. Any idea where this difference could come from?

johnjcamilleri avatar Sep 17 '21 12:09 johnjcamilleri

There is something really fishy. See what I get:

pgf.showExpr([], pgf.readExpr("'аb'")) "'аb'" pgf.showExpr([],pgf.readExpr("'ab'")) 'ab'

The only difference is the space after the comma. Do you get the same? I will try to investigate.

On Fri, 17 Sept 2021 at 14:07, John J. Camilleri @.***> wrote:

The marshalling and types and expressions in the Python bindings is almost done. One thing in particular which I don't understand is the treatment of single quotes in identifier names.

In the Haskell tests you have:

TestCase (assertEqual "unicode names 2" (Just "ab") (fmap (showExpr []) (readExpr "'ab'")))

Which indicates that the outer quotes around the identifier are dropped when not needed. In Python I have this test as:

def test_readExpr_efun_str_unicode_2():

assert pgf.showExpr([], pgf.readExpr("'аb'")) == "аb"

However this fails, but passes with ... == "'ab'". So in Python the outer quotes aren't being dropped. I would expect this behaviour to come from the runtime; we are both using pgf_print_expr and pgf_read_expr rather directly, and I don't see that you do anything in particular for quotes in the Haskell bindings. Any idea where this difference could come from?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GrammaticalFramework/gf-core/issues/130#issuecomment-921745286, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEYFSZATMPO5BY32DTH3MT3UCMVPXANCNFSM5BZSYMKQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

krangelov avatar Sep 17 '21 12:09 krangelov