On 01/27/2016 11:37 AM, Mike Bayer wrote: > > > On 01/27/2016 05:32 AM, Lele Gaifax wrote: >> Mike Bayer <mike...@zzzcomputing.com> writes: >> >>> Let's consider going with the third option I mentioned, I dont think >>> this will add too much expense if you could test this out please >> >> Here is the diff implementing the third option: it still need to touch also >> the C implementation, because that does not use the metadata keymap when the >> key is an integer.
OK my change has revealed a bug in my test suite that wasn't catching that my change fails. The C implementation uses the keymap for integers. Where are you looking that it doesn't ? > > awesome, you're right! I just committed the below (not using the > negative integer yet) > > any chance you could submit a PR at > https://github.com/zzzeek/sqlalchemy/ with the change + test for > negative indexes? > > > diff --git a/lib/sqlalchemy/engine/result.py > b/lib/sqlalchemy/engine/result.py > index cc4ac74..39f4fc5 100644 > --- a/lib/sqlalchemy/engine/result.py > +++ b/lib/sqlalchemy/engine/result.py > @@ -35,7 +35,10 @@ except ImportError: > > try: > from sqlalchemy.cresultproxy import BaseRowProxy > + _baserowproxy_usecext = True > except ImportError: > + _baserowproxy_usecext = False > + > class BaseRowProxy(object): > __slots__ = ('_parent', '_row', '_processors', '_keymap') > > @@ -210,11 +213,13 @@ class ResultMetaData(object): > context, cursor_description, result_columns, > num_ctx_cols, cols_are_ordered, textual_ordered) > > - # keymap indexes by integer index... > - self._keymap = dict([ > - (elem[0], (elem[3], elem[4], elem[0])) > - for elem in raw > - ]) > + self._keymap = {} > + if not _baserowproxy_usecext: > + # keymap indexes by integer index... > + self._keymap.update([ > + (elem[0], (elem[3], elem[4], elem[0])) > + for elem in raw > + ]) > > > > >> >> >> >> >> Given these differences between the two implementation, is there a way to >> explicitly perform the tests both with and without the C speedups? For now, I >> manually removed the *.so compiled thingies and re-executed the test suite. >> >> BTW, even if it seems unrelated to my diff, I'm seeing several >> errors/failures >> when running the tests under Py3 *without* C speedups: >> >> $ py.test test/ >> platform linux -- Python 3.5.1+, pytest-2.8.7, py-1.4.31, pluggy-0.3.1 -- >> /home/lele/wip/sqlalchemy/env/bin/python3 >> cachedir: .cache >> rootdir: /home/lele/wip/sqlalchemy, inifile: setup.cfg >> collected 7556 items >> >> test/aaa_profiling/test_compiler.py::CompileTest_sqlite_pysqlite::test_insert >> SKIPPED >> test/aaa_profiling/test_compiler.py::CompileTest_sqlite_pysqlite::test_select >> SKIPPED >> ... >> ============================= FAILURES ============================== >> ___________ QueuePoolTest.test_recycle_on_soft_invalidate ___________ >> >> Traceback (most recent call last): >> File "/home/lele/wip/sqlalchemy/test/engine/test_pool.py", line 1424, in >> test_recycle_on_soft_invalidate >> assert id(c3.connection) != c_id >> AssertionError: assert 139915463276696 != 139915463276696 >> + where 139915463276696 = id(<Mock id='139915463276696'>) >> + where <Mock id='139915463276696'> = <sqlalchemy.pool._ConnectionFairy >> object at 0x7f409b7cb8d0>.connection >> !!!!!!!!!!!!!! Interrupted: stopping after 25 failures !!!!!!!!!!!!!! >> === 1 failed, 1161 passed, 617 skipped, 24 error in 67.09 seconds === >> >> Py3 *with* C speedups, and Py2 in either contexts do not exhibit these >> errors/failures... >> >> Let me know which of the options you like most, or if there is anything else >> I >> can do/try. >> >> Thanks for the feedback, >> ciao, lele. >> > -- You received this message because you are subscribed to the Google Groups "sqlalchemy" group. To unsubscribe from this group and stop receiving emails from it, send an email to sqlalchemy+unsubscr...@googlegroups.com. To post to this group, send email to sqlalchemy@googlegroups.com. Visit this group at https://groups.google.com/group/sqlalchemy. For more options, visit https://groups.google.com/d/optout.
