cdfarrow / flexlibs

Library for accessing FieldWorks Language Explorer projects
Other
3 stars 4 forks source link

Fix TypeErrors etc. with pythonnet 3.0 #6

Closed MattGyverLee closed 1 year ago

MattGyverLee commented 2 years ago

FLEXTools recommends Python 3.8, which is no longer supported (neither is 3.9). FLExTools depends on FLEXLibs. Flexlibs expects 3.6-3.9 (but not 3.10).

Currently, a Windows user can't install a viable version of Python (3.8-3.9) for FLExTools without building Python from source.

Please update FlexLibs to support 3.10.

somelinguist commented 2 years ago

Support for Python above version 3.8 has been dependent on the release of Python.Net 3, which was just officially released last week: https://github.com/pythonnet/pythonnet/releases/tag/v3.0.0.

@cdfarrow, I can try to take a look at this this week to see if bumping versions will work.

MattGyverLee commented 2 years ago

Thanks!

somelinguist commented 2 years ago

@cdfarrow I just tried a simple bump to pythonnet 3.0.0 and ran pytest (still using python 3.8.10), which resulted in two failures (see below).

Based on your locking pythonnet to 2.5.2 in 3a0e5b3, you might already be aware of this. :)

My initial thoughts are that pythonnet 3.0.0 is not as forgiving in implicitly converting .net types.

There are a lot of places in the liblcm codebase where the internal implementation of a type adds extra properties/members that the public interface doesn't have. For example, internally, there is a LexEntry.ReferenceName that just points to LexEntry.HeadWord.Text, but there is no corresponding public ILexEntry.ReferenceName (the property ReferenceName is declared public, the the class LexEntry is marked as internal to the library).

In C#, I think you would have to cast ILexEntry instance specifically to LexEntry to access the ReferenceName property. Unfortunately, in this case, because the LexEntry type is internal, you can't actually cast to it because there's no way to access the name of the type LexEntry.

I think previous versions of pythonnet just didn't enforce this. There are lots of notes in the release notes for 3.0.0 about disabling several implicit conversions.

If I'm right about this, the only way I can think of to get around it for flexlibs (and flextools) is to specifically make sure each call is only to the public facing API that liblcm provides, which in many cases means just the interfaces (that start with I).

So for example, the definition of LexiconGetHeadword would need to be changed to the following:

def LexiconGetHeadword(self, entry):
        """
        Returns the headword for the entry
        """
        return entry.HeadWord.Text

Unfortunately, the liblcm codebase is a bit difficult to search through to find which classes, etc. are actually public. The best reference I've found is this Excel file which is available at https://software.sil.org/fieldworks/support/technical-documents/.

I might be able to help with this at some point, but I'm not sure when.


pytest results:

========================================================= test session starts =========================================================
platform win32 -- Python 3.8.10, pytest-7.1.3, pluggy-1.0.0
rootdir: C:\Users\Kevin\localcode\flexlibs
collected 5 items

flexlibs\tests\test_CustomFields.py F                                                                                            [ 20%]
flexlibs\tests\test_FLExInit.py .                                                                                                [ 40%] 
flexlibs\tests\test_FLExProject.py ..F                                                                                           [100%]

============================================================== FAILURES =============================================================== 
_____________________________________________________ TestSuite.test_WriteFields ______________________________________________________ 

self = <test_CustomFields.TestSuite testMethod=test_WriteFields>

    def test_WriteFields(self):
        fp = self._openProject()
>       flags_field = fp.LexiconGetEntryCustomFieldNamed(CUSTOM_FIELD)

flexlibs\tests\test_CustomFields.py:43:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
flexlibs\code\FLExProject.py:981: in LexiconGetEntryCustomFieldNamed
    return self.__FindCustomField(LexEntryTags.kClassId, fieldName)
flexlibs\code\FLExProject.py:956: in __FindCustomField
    for flid, name in self.__GetCustomFieldsOfType(classID):
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <flexlibs.code.FLExProject.FLExProject object at 0x00000144DF9BD3A0>, classID = 5002

    def __GetCustomFieldsOfType(self, classID):
        """
        Generator for finding all the custom fields at Sense or Entry level.
        Returns tuples of (flid, label)
        """

        # The MetaDataCache defines the project structure: we can
        # find the custom fields in here.
        mdc = self.project.MetaDataCacheAccessor
>       for flid in mdc.GetFields(classID, False, -1):
E       TypeError: No method matches given arguments for IFwMetaDataCache.GetFields: (<class 'int'>, <class 'bool'>, <class 'int'>)     

flexlibs\code\FLExProject.py:951: TypeError
__________________________________________________ TestFLExProject.test_ReadLexicon ___________________________________________________ 

self = <test_FLExProject.TestFLExProject testMethod=test_ReadLexicon>

    def test_ReadLexicon(self):
        fp = FLExProject()
        projectName = AllProjectNames()[0]
        try:
            fp.OpenProject(projectName,
                           writeEnabled = False)
        except Exception as e:
            self.fail("Exception opening project %s" % projectName)

        # Traverse the whole lexicon
        for lexEntry in fp.LexiconAllEntries():
>           self.assertIsInstance(fp.LexiconGetHeadword(lexEntry), str)

flexlibs\tests\test_FLExProject.py:46:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <flexlibs.code.FLExProject.FLExProject object at 0x00000144DF9BDAF0>
entry = <SIL.LCModel.ILexEntry object at 0x00000144C708B800>

    def LexiconGetHeadword(self, entry):
        """
        Returns the headword for the entry
        """
>       return entry.ReferenceName
E       AttributeError: 'ILexEntry' object has no attribute 'ReferenceName'
somelinguist commented 2 years ago

I'm not sure what's going on with TypeError: No method matches given arguments for IFwMetaDataCache.GetFields: (<class 'int'>, <class 'bool'>, <class 'int'>). As far as I can tell, that should match the public definition in liblcm at

https://github.com/sillsdev/liblcm/blob/8f04ab5b314e9fa4cfacd4c481b82bc1ee2d6a91/src/SIL.LCModel/Infrastructure/IFwMetaDataCacheManaged.cs#L46

cdfarrow commented 2 years ago

Support for Python above version 3.8 has been dependent on the release of Python.Net 3, which was just officially released last week: https://github.com/pythonnet/pythonnet/releases/tag/v3.0.0.

@somelinguist Yes, I've had several reports of people installing FlexTools in the last week, and it hasn't worked because of the pythonnet v3.0.0 release. Which is why I made a quick update pinning to v2.5.2.

Thanks so much for your research. If you're able to help figure out what changes we need to make, that will be great.

@MattGyverLee We've been constrained to <= Python 3.8 because of this dependency. With v3 out we can now work to support it and Python 3.9 & 3.10, however, since pythonnet v3 is brand new, I would be wary of making that the recommendation too soon.

cdfarrow commented 2 years ago

Currently, a Windows user can't install a viable version of Python (3.8-3.9) for FLExTools without building Python from source.

Thanks for raising this issue. BTW, you can still get Python 3.8.10 here, (although it doesn't have the latest security patches).

cdfarrow commented 1 year ago

I've updated flexlibs to v1.2.2, which supports Python 3.8 through to 3.12 (using pythonnet 3.0.3)