[issue9523] Improve dbm modules
Changes by Jesús Cea Avión j...@jcea.es: -- nosy: +jcea ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
ysj.ray ysj@gmail.com added the comment: Sorry, previous patch(issue_9523_4.diff) missed a file(Lib/test/dbm_tests.py) Here is an intact one. -- Added file: http://bugs.python.org/file21769/issue_9523_5.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Ray.Allen ysj@gmail.com added the comment: I tried to work out a doc patch for 3.2 to mention the limitation api: the missing methods compared with dict and the imperfect methods(keys(), items()) of collections.MutableMapping. Here is it. -- Added file: http://bugs.python.org/file21369/issue_9523_3.2_doc_patch.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Ray.Allen ysj@gmail.com added the comment: Updated patch: 1, Changes follows review comments: http://codereview.appspot.com/4185044/. Thanks eric! 2, Make Objects/dictobject.c:all_contained_in() a common useful limited api Object/abstract.c:_PyObject_AllContainedIn() for the purpose of re-usage in Modules/_gdbmmodule.c and Modules/_dbmmodule.c. Not sure if this is proper. I will ask somebody with C knowledge to do a review on the C code. -- Added file: http://bugs.python.org/file21355/issue_9523_3.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Changes by Nick Coghlan ncogh...@gmail.com: -- nosy: +ncoghlan ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Ray.Allen ysj@gmail.com added the comment: I think the patch will not be suitable for 3.1 and 3.2 Yes, it changes some api(e.g keys()), which may introduces compatibility issues. so there should be a doc patch to mention the limitations of the dbm API (keys() returning a list and all that). Do you mean a doc patch for 3.2 which mentions the missing or imperfect methods of collections.MutableSequence()? e.g keys() not returns a KeysView but a list instead, update() is missing -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Éric Araujo mer...@netwok.org added the comment: Yes, I mean exactly that. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Éric Araujo mer...@netwok.org added the comment: I think the patch will not be suitable for 3.1 and 3.2, so there should be a doc patch to mention the limitations of the dbm API (keys() returning a list and all that). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Ray.Allen ysj@gmail.com added the comment: An updated patch, based on latest several reviews on http://codereview.appspot.com/4185044/ update: 1, Refactoring the common tests between test_dbm_gnu and test_dbm_ndbm. 2, Move the abc registering in Lib/dbm/ndbm.py and Lib/dbm/gnu.py. 3, Other changes pointed out in review comments. -- Added file: http://bugs.python.org/file20783/issue_9523.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Changes by Ray.Allen ysj@gmail.com: Removed file: http://bugs.python.org/file20771/issue_9523.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Ray.Allen ysj@gmail.com added the comment: Thanks! Here is my updated patch: 1, Now the dbm view objects are the same as dict view objects, which are in conformity with collections.KeysView, ValuesView and ItemsView. 2, I register all these abcs explicitly because these abcs have not __subclasshook__() method so they can't check api conformance(at lease exist) through isinstance(). I could not make sure api conformance except testing each method I find in abc explicitly. And my test_abc() is just to test the registering. 3, Other updates which are from comments I wrote newly on codereview: http://codereview.appspot.com/4185044/ -- Added file: http://bugs.python.org/file20771/issue_9523.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Changes by Ray.Allen ysj@gmail.com: Removed file: http://bugs.python.org/file20751/issue_9523.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Éric Araujo mer...@netwok.org added the comment: Here is my updated patch: You don’t have to attach a file here, just update the codereview page instead. Or maybe you can’t because I created the page? 1, Now the dbm view objects are the same as dict view objects, which are in conformity with collections.KeysView, ValuesView and ItemsView. Great! Just to make sure: How do you know that the view objects are compliant? Do you test all the methods documented for the ABCs? 2, I register all these abcs explicitly because these abcs have not __subclasshook__() method so they can't check api conformance(at lease exist) through isinstance(). I could not make sure api conformance except testing each method I find in abc explicitly. And my test_abc() is just to test the registering. Thank you for repeating that many times and politely: I was indeed wrong. I went back to PEP 3119 to read again about __instancecheck__ and __subclasscheck__, then experimented in a shell and was surprised. I have been misunderstanding one thing: issubclass(cls, abc) does not return true automatically if cls provides the methods, it’s entirely up to the ABC to check methods or do something else in its __subclasscheck__ or __instancecheck__ methods. (I should have known better, I was in a discussion about adding that very feature on python-ideas and #9731!) After another bit of experimentation with dict views and collections ABCs, I finally understand that you have to register your view classes. Thanks for letting me correct my misunderstanding. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Changes by Ray.Allen ysj@gmail.com: Removed file: http://bugs.python.org/file18402/issue8634.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Changes by Ray.Allen ysj@gmail.com: Removed file: http://bugs.python.org/file19987/issue_9523.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Changes by Ray.Allen ysj@gmail.com: Removed file: http://bugs.python.org/file20726/issue_9523.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Ray.Allen ysj@gmail.com added the comment: 1. Make keys(), values(), items() methods return view object for ndbm, gdbm and dumb objects. I following the codes in dictobject.c. Did you have to copy the code? Isn’t it possible to somehow reuse it? I feel not so easy to reuse the code, there could be several differences in the c code. Resuing the code may make the code more complecated. But if someone could find a better way to reuse the code, it would be nice. The keysview object support len(), in operator, and iteratable, while valuesview and itemsview object only support len() and iteratable. That does not seem to comply with the definition of dict views. Oh, yes, I missed the rich compare functions and isdisjoint() method of view objects. Do the views yield elements in the same order? (In a dict, iteration order is undefined but consistent between the various views, IIUC.) gdbm and dumb views yield elements in the same order, ndbm views doesn't. I missed it. 3. Remove dumb's keys() method which calls self._index.keys() since it is unnecessary. Does dumb have no keys method then? Yes, it does. Its keys() method is provided by Mapping abc already. Here is the updated patch: 1. Add rich compare functions and disjoint() method to dbm view objects to make them as MappingView objects, and add abc registration for them. 2. Make ndbm view objects yield elements in the same order. 3. Other changes during to the codeview: http://codereview.appspot.com/4185044/ -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Changes by Ray.Allen ysj@gmail.com: Added file: http://bugs.python.org/file20751/issue_9523.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Éric Araujo mer...@netwok.org added the comment: Add rich compare functions and disjoint() method to dbm view objects to make them as MappingView objects, and add abc registration for them. I’d prefer you not to register them, but test isinstance(keys(), KeysView), so that we’re sure no method is missing. (Does not validate behavior, but it’s a starting point.) Other comments on Rietveld (the code review site). BTW, if you’re posting updated patches on Rietveld you can remove patches from this bug report, so that people don’t review old patches. It would also be simpler to keep all discussion there :) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Éric Araujo mer...@netwok.org added the comment: Closed #5736 as superseded. Please make sure the comments there about the peculiar API of gdbm don’t come up or are addressed in your patch. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Ray.Allen ysj@gmail.com added the comment: Thanks eric for reviewing my patch! And thanks for you suggestions. I'm following them. I don’t know if you should use a plain set or a collections.ItemsView here. In dict objects, KeysView and ValuesView are set-like objects with added behavior, for example they yield their elements in the same order. Yes you are right. I think returning a view object is better than returning a set. Here is the updated patch. It updates: 1. Make keys(), values(), items() methods return view object for ndbm, gdbm and dumb objects. I following the codes in dictobject.c. The keysview object support len(), in operator, and iteratable, while valuesview and itemsview object only support len() and iteratable. 2. Removing doc changes: The object returned by :func:`.open` supports the same basic functionality as -dictionaries +:mod:`collection`.MutableMapping which is mentioned in eric's comment. 3. Remove dumb's keys() method which calls self._index.keys() since it is unnecessary. 4. Using more specialized assertXxx methods in test cases. 5. Remove the values() and items() method are not supported in Doc/library/dbm.rst. See #5736 for a patch adding iteration support. If the patch attached to his report supersedes the other one, I’ll close the other bug as duplicate. #5736 's patch for adding iteration to ndbm and gdbm modules simple calling PyObject_GetIter(dbm_keys(dbm, NULL)) for both gdbm and ndbm, but I feel it's better to create a seperate iterator object for gdbm objects. -- Added file: http://bugs.python.org/file20726/issue_9523.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Éric Araujo mer...@netwok.org added the comment: 1. Make keys(), values(), items() methods return view object for ndbm, gdbm and dumb objects. I following the codes in dictobject.c. Did you have to copy the code? Isn’t it possible to somehow reuse it? The keysview object support len(), in operator, and iteratable, while valuesview and itemsview object only support len() and iteratable. That does not seem to comply with the definition of dict views. Do the views yield elements in the same order? (In a dict, iteration order is undefined but consistent between the various views, IIUC.) 3. Remove dumb's keys() method which calls self._index.keys() since it is unnecessary. Does dumb have no keys method then? 4. Using more specialized assertXxx methods in test cases. See my comments on http://codereview.appspot.com/4185044/ #5736 's patch for adding iteration to ndbm and gdbm modules simple calling PyObject_GetIter(dbm_keys(dbm, NULL)) for both gdbm and ndbm, but I feel it's better to create a seperate iterator object for gdbm objects. Okay, so I shall close the other bug report, indicating it’s superseded by your patch. I can’t judge the C code; maybe Raymond or Daniel will. They’re also experts about collections and ABCs, so they’ll be able to confirm or infirm what I’ve said about dict views and the registration stuff (on codereview). -- nosy: +stutzbach ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Éric Araujo mer...@netwok.org added the comment: Thank you for working on a patch, especially with such comprehensive tests. The object returned by :func:`.open` supports the same basic functionality as -dictionaries +:mod:`collection`.MutableMapping The previous text was okay, I wouldn’t have changed it. def items(self): +return set([(key, self[key]) for key in self._index.keys()]) I don’t know if you should use a plain set or a collections.ItemsView here. In dict objects, KeysView and ValuesView are set-like objects with added behavior, for example they yield their elements in the same order. Raymond, can you comment? Style remarks: you can iter without calling _index.keys(); you can avoid the intermediary list (IOW, remove enclosing [ and ]). In the tests, you can use specialized methods like assertIn and assertIsNone, they remove some boilerplate and can give better error output. I can’t review the C code. :) -- nosy: +rhettinger versions: +Python 3.3 -Python 3.2 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Éric Araujo mer...@netwok.org added the comment: See #5736 for a patch adding iteration support. If the patch attached to his report supersedes the other one, I’ll close the other bug as duplicate. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Ray.Allen ysj@gmail.com added the comment: Here is the updated patch, which fixed: 1. remove get() method of gdbm since issue6045 has already add it. 2. method keys() and items() of dbm object return set instead of list. Since pep3119 said keys() and items() should return collections.Set and set is a collections.Set. 3. add update() method to dbm object, which follows implementation in collections.MutableMapping.update(). -- Added file: http://bugs.python.org/file19987/issue_9523.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Ray.Allen ysj@gmail.com added the comment: Oh, yes. I noticed that the pep3119 defines return value of method MutableMapping.keys() as Set, as well as method items(). So the implementation of dumb.keys() and dump.items() are not correct since they all return lists while the class inherits MutableMapping. The implementations in my patch should also be corrected since I made the same mistake. Besides, since issue6045 has already added get(), I need to update my patch. I will do it later. And who can tell the specification of MutableMapping.update()? The pep3119 lacks of it. Should I follow the implementation in the ABC class MutableMapping? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
R. David Murray rdmur...@bitdance.com added the comment: I believe that in the absence of other documentation the ABC is considered authoritative. -- nosy: +r.david.murray ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Éric Araujo mer...@netwok.org added the comment: In 3.2, objects return by dbm.dumb.open implement MutableMapping with incorrect semantics: keys return a list, iterkeys exist, etc. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Changes by Éric Araujo mer...@netwok.org: -- nosy: +eric.araujo title: Improve dbm module - Improve dbm modules ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9523] Improve dbm modules
Changes by Éric Araujo mer...@netwok.org: -- nosy: +Kain94, georg.brandl ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9523 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
