Update changelog #25

rahulporuri · 2019-01-04T05:15:17Z

in prep for the 4.3.0 release of codetools

in prep for the 4.3.0 release of codetools modified: CHANGES.txt

rahulporuri · 2019-01-04T06:13:19Z

the python 2.7 job failed with the error

ERROR: Checking if the data persists correctly when saving and loading back
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python2.7.14/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/travis/build/enthought/codetools/codetools/contexts/tests/multi_context_test_case.py", line 126, in test_persistence
    m.save(f)
  File "/home/travis/build/enthought/codetools/codetools/contexts/data_context.py", line 227, in save
    sweet_pickle.dump(self, file_object, 1)
  File "/home/travis/virtualenv/python2.7.14/lib/python2.7/site-packages/apptools/sweet_pickle/__init__.py", line 131, in dump
    return d(obj, file, protocol)
  File "/home/travis/virtualenv/python2.7.14/lib/python2.7/copy_reg.py", line 70, in _reduce_ex
    raise TypeError, "can't pickle %s objects" % base.__name__
TypeError: can't pickle instancemethod objects
----------------------------------------------------------------------

rahulporuri · 2019-01-25T10:44:58Z

Note :

codetools doesn't have a CI infrastructure similar to other ETS projects i.e. the etstool module used to setup CI
codetools doesn't explicitly specify dependency versions to be used by the CI. Not only that, there are a number of packages that are installed from their respective git master branches e.g. scimath, traits.
codetools only tests on linux at the moment and osx/win CI jobs have not been setup.

see .travis.yml for the existing CI setup process and travis-ci-requirements.txt for the requirements/versions.

Because dependencies are installed directly from the master branches, the CI job in this PR failed, even though it didn't modify any of the code or change the version of any dependency.

rahulporuri · 2019-02-01T19:31:00Z

in order to reproduce the issue on the command line -

from io import BytesIO

from codetools.contexts.data_context import DataContext
from codetools.contexts.multi_context import MultiContext


dc1=DataContext(name='temp1', subcontext={'a': 1})
dc2 = DataContext(name='temp2', subcontext={'b': 2})
mc = MultiContext(dc1, dc2, name='woah')
f = BytesIO()
dc1.save(f)
mc.save(f)

Saving the data context works as expected and we see the above error when we try saving/pickling the multi context.

rahulporuri · 2019-02-01T19:50:49Z

This is very interesting. MultiContext.save eventually passes the instance/self to apptools.sweet_pickle.dump. apptools.sweet_pickle hasn't been modified in 4 years and it mostly just passes the instance to cPickle.dump.

So, the only change is the instance itself and how cPickle.dump tries serializing the instance. I think.

rahulporuri · 2019-02-02T07:40:20Z

So. The changes introduced to __getstate__ in PR enthought/traits#373 are the cause of the failure. Temporarily commenting out those changes to __getstate__ fixes the issue. Investigating why now.

The final stack where we get the error is :

Details

> /home/rporuri/.edm/envs/codet/lib/python2.7/site-packages/traits/traits.py(510)__reduce_ex__()
    508 
    509     def __reduce_ex__(self, protocol):
--> 510         return (__newobj__, (self.__class__, 0), self.__getstate__())
    511 
    512     # ---------------------------------------------------------------------------

ipdb> n
--Call--
> /home/rporuri/.edm/envs/codet/lib/python2.7/copy_reg.py(59)_reduce_ex()
     57 # Python code for object.__reduce_ex__ for protocols 0 and 1
     58 
---> 59 def _reduce_ex(self, proto):
     60     assert proto < 2
     61     for base in self.__class__.__mro__:

ipdb> n
> /home/rporuri/.edm/envs/codet/lib/python2.7/copy_reg.py(60)_reduce_ex()
     58 
     59 def _reduce_ex(self, proto):
---> 60     assert proto < 2
     61     for base in self.__class__.__mro__:
     62         if hasattr(base, '__flags__') and not base.__flags__ & _HEAPTYPE:

ipdb> n
> /home/rporuri/.edm/envs/codet/lib/python2.7/copy_reg.py(61)_reduce_ex()
     59 def _reduce_ex(self, proto):
     60     assert proto < 2
---> 61     for base in self.__class__.__mro__:
     62         if hasattr(base, '__flags__') and not base.__flags__ & _HEAPTYPE:
     63             break

ipdb> n
> /home/rporuri/.edm/envs/codet/lib/python2.7/copy_reg.py(62)_reduce_ex()
     60     assert proto < 2
     61     for base in self.__class__.__mro__:
---> 62         if hasattr(base, '__flags__') and not base.__flags__ & _HEAPTYPE:
     63             break
     64     else:

ipdb> n
> /home/rporuri/.edm/envs/codet/lib/python2.7/copy_reg.py(63)_reduce_ex()
     61     for base in self.__class__.__mro__:
     62         if hasattr(base, '__flags__') and not base.__flags__ & _HEAPTYPE:
---> 63             break
     64     else:
     65         base = object # not really reachable

ipdb> n
> /home/rporuri/.edm/envs/codet/lib/python2.7/copy_reg.py(66)_reduce_ex()
     64     else:
     65         base = object # not really reachable
---> 66     if base is object:
     67         state = None
     68     else:

ipdb> n
> /home/rporuri/.edm/envs/codet/lib/python2.7/copy_reg.py(69)_reduce_ex()
     67         state = None
     68     else:
---> 69         if base is self.__class__:
     70             raise TypeError, "can't pickle %s objects" % base.__name__
     71         state = base(self)

ipdb> n
> /home/rporuri/.edm/envs/codet/lib/python2.7/copy_reg.py(70)_reduce_ex()
     68     else:
     69         if base is self.__class__:
---> 70             raise TypeError, "can't pickle %s objects" % base.__name__
     71         state = base(self)
     72     args = (self.__class__, base, state)

ipdb> n
TypeError: TypeErro...bjects",)
> /home/rporuri/.edm/envs/codet/lib/python2.7/copy_reg.py(70)_reduce_ex()
     68     else:
     69         if base is self.__class__:
---> 70             raise TypeError, "can't pickle %s objects" % base.__name__
     71         state = base(self)
     72     args = (self.__class__, base, state)

ipdb> n
--Return--
None
> /home/rporuri/.edm/envs/codet/lib/python2.7/copy_reg.py(70)_reduce_ex()
     68     else:
     69         if base is self.__class__:
---> 70             raise TypeError, "can't pickle %s objects" % base.__name__
     71         state = base(self)
     72     args = (self.__class__, base, state)

ipdb> n
TypeError: TypeErro...bjects",)
> /home/rporuri/.edm/envs/codet/lib/python2.7/site-packages/apptools/sweet_pickle/__init__.py(131)dump()
    129     _flush_traits(obj)
    130     from cPickle import dump as d
--> 131     return d(obj, file, protocol)
    132 
    133 def dumps(obj, protocol=2):

ipdb> d
None
> /home/rporuri/.edm/envs/codet/lib/python2.7/copy_reg.py(70)_reduce_ex()
     68     else:
     69         if base is self.__class__:
---> 70             raise TypeError, "can't pickle %s objects" % base.__name__
     71         state = base(self)
     72     args = (self.__class__, base, state)

ipdb> base
<type 'instancemethod'>
ipdb> self.__class__
<type 'instancemethod'>
ipdb> self
<bound method Supports.create_default_value of <traits.trait_types.Supports object at 0x7f7658fc39d0>>

rahulporuri · 2019-02-07T10:59:37Z

As you can see from the CI runs, this issue is Python 2 only. Note sure yet why that's the case.

rkern · 2019-02-11T19:10:34Z

Pickling instancemethods is a new feature in Python 3: https://bugs.python.org/issue23611

rkern · 2019-02-12T01:20:43Z

enthought/traits#373 breaks pickling on Python 2 for many classes. Any Instance trait (of which Supports is a subclass) with a factory default (i.e. with args=… or kw=…) stores an instancemethod from BaseInstance on itself: https://github.com/enthought/traits/blob/master/traits/trait_types.py#L3139-L3143

This shows up in MultiContext, which declares a subcontexts = List(Supports) trait on the class, because when it attaches listeners to subcontexts, Traits copies over the CTrait from the class trait dict over to the instance trait dict in order to attach the instance-specific notification handlers.

This specific problem can mostly be gotten around if enthought/traits#373 were to remove traits from the instance trait dict that look like they were so copied (e.g. check the identity of the .handlers on the CTraits). The intent of enthought/traits#373 was to only pick up those traits that were added with add_trait(), but HasTraits._instance_traits() returns a dict with both kinds.

rkern · 2019-02-12T01:23:31Z

However, even with that changed, it would still fail with explicitly-added Instance traits (and who knows which others?).

mdickinson · 2019-02-12T09:39:38Z

However, even with that changed, it would still fail with explicitly-added Instance traits (and who knows which others?).

Okay, sounds like we need a rethink and changes in Traits, not just here.

…te-changelog

- with info on recent PRs that were merged - with note about broken `MultiContext` instance pickling on Python 2 modified: CHANGES.txt

codecov-io · 2019-02-18T08:17:26Z

Codecov Report

❗ No coverage uploaded for pull request base (master@8341522). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff            @@
##             master     #25   +/-   ##
========================================
  Coverage          ?   44.7%           
========================================
  Files             ?      72           
  Lines             ?    4586           
  Branches          ?     970           
========================================
  Hits              ?    2050           
  Misses            ?    2346           
  Partials          ?     190

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8341522...9574a23. Read the comment docs.

mdickinson

LGTM

DOC : update changelog

d4528ea

in prep for the 4.3.0 release of codetools modified: CHANGES.txt

rahulporuri mentioned this pull request Feb 1, 2019

Replace usage of adapt from traits.protocols.api #27

Merged

rahulporuri mentioned this pull request Feb 15, 2019

Skip persistence test for MultiContext - Py2 #28

Merged

Merge branch 'master' of github.com:enthought/codetools into doc/upda…

7ca77f0

…te-changelog

rahulporuri mentioned this pull request Feb 18, 2019

Pickling MultiContext instances is broken on Python 2 #30

Closed

DOC : update changelog

9574a23

- with info on recent PRs that were merged - with note about broken `MultiContext` instance pickling on Python 2 modified: CHANGES.txt

mdickinson approved these changes Feb 18, 2019

View reviewed changes

mdickinson mentioned this pull request Feb 18, 2019

Pickling on Python 2 broken for many classes enthought/traits#452

Closed

rahulporuri merged commit 61ae68e into master Feb 18, 2019

rahulporuri deleted the doc/update-changelog branch February 18, 2019 08:31

Update changelog #25

Update changelog #25

Uh oh!

Conversation

rahulporuri commented Jan 4, 2019

Uh oh!

rahulporuri commented Jan 4, 2019

Uh oh!

rahulporuri commented Jan 25, 2019

Uh oh!

rahulporuri commented Feb 1, 2019

Uh oh!

rahulporuri commented Feb 1, 2019

Uh oh!

rahulporuri commented Feb 2, 2019

Uh oh!

rahulporuri commented Feb 7, 2019

Uh oh!

rkern commented Feb 11, 2019

Uh oh!

rkern commented Feb 12, 2019

Uh oh!

rkern commented Feb 12, 2019

Uh oh!

mdickinson commented Feb 12, 2019

Uh oh!

codecov-io commented Feb 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mdickinson left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-io commented Feb 18, 2019 •

edited

Loading