|
|
|
Example works here.
python -m doctest trytondasmodule.rst:
ok
1 items passed all tests:
27 tests in trytondasmodule.rst
27 tests in 1 items.
27 passed and 0 failed.
Test passed.
Patch Set 1 #
Total comments: 8
Patch Set 2 : Updated to run directly from sources #
Total comments: 44
Total messages: 5
|
udono
Looks good. http://codereview.appspot.com/2552041/diff/1/doc/topics/index.rst File doc/topics/index.rst (right): http://codereview.appspot.com/2552041/diff/1/doc/topics/index.rst#newcode16 doc/topics/index.rst:16: trytondasmodule here maybe I would use a ...
|
15 years, 2 months ago (2010年10月18日 16:30:00 UTC) #1 | ||||||||||||||||||||||||||||||
Looks good. http://codereview.appspot.com/2552041/diff/1/doc/topics/index.rst File doc/topics/index.rst (right): http://codereview.appspot.com/2552041/diff/1/doc/topics/index.rst#newcode16 doc/topics/index.rst:16: trytondasmodule here maybe I would use a prefix/path like importexport/trytondasmodule scripts/trytondasmodule ... Because this topic does not fit this well into the API specific parts. http://codereview.appspot.com/2552041/diff/1/doc/topics/trytondasmodule.rst File doc/topics/trytondasmodule.rst (right): http://codereview.appspot.com/2552041/diff/1/doc/topics/trytondasmodule.rst#n... doc/topics/trytondasmodule.rst:7: Tryton Code as Library http://codereview.appspot.com/2552041/diff/1/doc/topics/trytondasmodule.rst#n... doc/topics/trytondasmodule.rst:9: a trytond/tryton instance on the same machine providing the internal API to there is no need of a running trytond. The trytons code library instantiate a running server by itself. http://codereview.appspot.com/2552041/diff/1/doc/topics/trytondasmodule.rst#n... doc/topics/trytondasmodule.rst:46: >>> CONFIG.load() After this I would overwrite the loaded config to database type sqlite and :memory: for passing the doctest. http://codereview.appspot.com/2552041/diff/1/doc/topics/trytondasmodule.rst#n... doc/topics/trytondasmodule.rst:59: >>> dbname = 'test' :memory:
http://codereview.appspot.com/2552041/diff/1/doc/topics/index.rst File doc/topics/index.rst (right): http://codereview.appspot.com/2552041/diff/1/doc/topics/index.rst#newcode16 doc/topics/index.rst:16: trytondasmodule On 2010年10月18日 16:30:00, udono wrote: > here maybe I would use a prefix/path like > importexport/trytondasmodule > scripts/trytondasmodule > ... > > Because this topic does not fit this well into the API specific parts. Agreed, it#s just what ced proposed. If he agrees with prefix scripting or scripts, I am happy with it. http://codereview.appspot.com/2552041/diff/1/doc/topics/trytondasmodule.rst File doc/topics/trytondasmodule.rst (right): http://codereview.appspot.com/2552041/diff/1/doc/topics/trytondasmodule.rst#n... doc/topics/trytondasmodule.rst:9: a trytond/tryton instance on the same machine providing the internal API to On 2010年10月18日 16:30:00, udono wrote: > there is no need of a running trytond. The trytons code library instantiate a > running server by itself. It doesn't say, that it connects to some other instance, but that it provides 'direct access to a trytond/tryton instance on the same machine'. http://codereview.appspot.com/2552041/diff/1/doc/topics/trytondasmodule.rst#n... doc/topics/trytondasmodule.rst:46: >>> CONFIG.load() On 2010年10月18日 16:30:00, udono wrote: > After this I would overwrite the loaded config to database type sqlite and > :memory: for passing the doctest. Thanks, good idea. I will try if it is possible to set PYTHONPATH as required. Then it can make sense to run this doctest automatically at all. See also the codereview description.
http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rst File doc/topics/trytondasmodule.rst (right): http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:5: Tryton Code as Library Why Library? http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:9: a trytond/tryton instance on the same machine providing the internal API to It is not access to instance but to data http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:17: Interaction with a Tryton server (trytond) with a simple import It is not to a server. http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:22: can do something like export PYTHONPATH=/path/to/trytond/:$PYTHONPATH . I don't see why speaking about this. This is general concept from Python http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:26: config file of the server (but it will slow down the cache mechanisms). You should just say that multi_server must be set to True. http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:28: modules with the profiling tools of Python. Why? http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:30: database only after a cursor.commit() is issued. Once again it is linked to PEPxxx which is database connection. http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:38: >>> #! /usr/bin/env python No need of shebang for Python shell http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:43: ... sys.path.insert(0, os.path.dirname(DIR)) I find this code is not clear for tutorial http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:73: >>> USER = 1 I will not use uppercase here http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:87: Start a transaction and install the module needed for our example I find that installing a module doesn't show well the power of this feature. http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:89: >>> MODULE = 'party' Why upper case? http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:102: ... cursor.commit() Why do you commit http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:104: ... cursor.commit() idem http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:118: 1 This test is not reliable http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:122: >>> Cache.resets(DBNAME) You must reset Cache after each commit
http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rst File doc/topics/trytondasmodule.rst (right): http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:5: Tryton Code as Library On 2010年10月25日 14:09:42, ced wrote: > Why Library? Took it from http://code.google.com/p/tryton/wiki/HowToUseTrytondAsAModule . http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:9: a trytond/tryton instance on the same machine providing the internal API to On 2010年10月25日 14:09:42, ced wrote: > It is not access to instance but to data For me it is access to data by means of an API. And trytondasmodule is access to that API. http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:17: Interaction with a Tryton server (trytond) with a simple import On 2010年10月25日 14:09:42, ced wrote: > It is not to a server. ?? If trytond is not a server why do you write http://news.tryton.org/2010/09/some-news-from-development.html ? http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:22: can do something like export PYTHONPATH=/path/to/trytond/:$PYTHONPATH . On 2010年10月25日 14:09:42, ced wrote: > I don't see why speaking about this. > This is general concept from Python It is a preliminary with which not each user is familiar. Answering those questions in docs prevents people from asking on mailing list or IRC. http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:28: modules with the profiling tools of Python. On 2010年10月25日 14:09:42, ced wrote: > Why? Dont't understand. http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:30: database only after a cursor.commit() is issued. On 2010年10月25日 14:09:42, ced wrote: > Once again it is linked to PEPxxx which is database connection. This doc is intended to provide a code snippet to show an example of the usage of trytond as module, not a reference. So same as above applies: help the users to avoid errors. Makes especially sense with the following sentence. http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:38: >>> #! /usr/bin/env python On 2010年10月25日 14:09:42, ced wrote: > No need of shebang for Python shell Not here, but if copied into a script it will be necessary. I will omit it. http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:43: ... sys.path.insert(0, os.path.dirname(DIR)) On 2010年10月25日 14:09:42, ced wrote: > I find this code is not clear for tutorial I can elaborate, that here the path to trytond is set by other means than environment. http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:73: >>> USER = 1 On 2010年10月25日 14:09:42, ced wrote: > I will not use uppercase here http://hg.tryton.org/trytond/file/5749fbc08672/trytond/tests/test_tryton.py#l44 http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:87: Start a transaction and install the module needed for our example On 2010年10月25日 14:09:42, ced wrote: > I find that installing a module doesn't show well the power of this feature. The module is installed to fulfill preliminaries for the following access to objects of party. http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:89: >>> MODULE = 'party' On 2010年10月25日 14:09:42, ced wrote: > Why upper case? See above. http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:102: ... cursor.commit() On 2010年10月25日 14:09:42, ced wrote: > Why do you commit http://hg.tryton.org/trytond/file/5749fbc08672/trytond/tests/test_tryton.py#l123 http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:104: ... cursor.commit() On 2010年10月25日 14:09:42, ced wrote: > idem http://hg.tryton.org/trytond/file/5749fbc08672/trytond/tests/test_tryton.py#l125 http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:118: 1 On 2010年10月25日 14:09:42, ced wrote: > This test is not reliable It is a simple check on the expected result. This doc is not a test. http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:122: >>> Cache.resets(DBNAME) On 2010年10月25日 14:09:42, ced wrote: > You must reset Cache after each commit Thanks!
http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rst File doc/topics/trytondasmodule.rst (right): http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:5: Tryton Code as Library On 2010年10月25日 21:38:01, yangoon wrote: > On 2010年10月25日 14:09:42, ced wrote: > > Why Library? > > Took it from http://code.google.com/p/tryton/wiki/HowToUseTrytondAsAModule . It will be more consisting to have the same title than the file name. http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:9: a trytond/tryton instance on the same machine providing the internal API to On 2010年10月25日 21:38:01, yangoon wrote: > On 2010年10月25日 14:09:42, ced wrote: > > It is not access to instance but to data > > For me it is access to data by means of an API. And trytondasmodule is access to > that API. Ok. But it is not accessing to an instance of trytond. So I propose this: Using trytond as a module allow to access to data by using the internal Python API. http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:17: Interaction with a Tryton server (trytond) with a simple import On 2010年10月25日 21:38:01, yangoon wrote: > On 2010年10月25日 14:09:42, ced wrote: > > It is not to a server. > > ?? > If trytond is not a server why do you write > http://news.tryton.org/2010/09/some-news-from-development.html > ? I don't say that. I said that you don't interact with a server. You use the same code. http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:22: can do something like export PYTHONPATH=/path/to/trytond/:$PYTHONPATH . On 2010年10月25日 21:38:01, yangoon wrote: > On 2010年10月25日 14:09:42, ced wrote: > > I don't see why speaking about this. > > This is general concept from Python > > It is a preliminary with which not each user is familiar. Answering those > questions in docs prevents people from asking on mailing list or IRC. This should not be part of Tryton documentation. Just point to the Python doc: http://docs.python.org/tutorial/modules.html#the-module-search-path http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:28: modules with the profiling tools of Python. On 2010年10月25日 21:38:01, yangoon wrote: > On 2010年10月25日 14:09:42, ced wrote: > > Why? > > Dont't understand. I don't why this sentence is there. There is no example of this usage and personally I don't see why it is easier for that. http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:30: database only after a cursor.commit() is issued. On 2010年10月25日 21:38:01, yangoon wrote: > On 2010年10月25日 14:09:42, ced wrote: > > Once again it is linked to PEPxxx which is database connection. > > This doc is intended to provide a code snippet to show an example of the usage > of trytond as module, not a reference. So same as above applies: help the users > to avoid errors. Makes especially sense with the following sentence. We must not duplicate documentation because it is too hard to maintain it. If you really want to describe the usage of database cursor in Python, just link to http://www.python.org/dev/peps/pep-0249/ http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:31: * It is possible to do safe tests just by avoiding the cursor.commit() call. Same as above. http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:38: >>> #! /usr/bin/env python On 2010年10月25日 21:38:01, yangoon wrote: > On 2010年10月25日 14:09:42, ced wrote: > > No need of shebang for Python shell > > Not here, but if copied into a script it will be necessary. I will omit it. Neither, you can run it with `python script.py` You will never see this in the Python documentation. http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:43: ... sys.path.insert(0, os.path.dirname(DIR)) On 2010年10月25日 21:38:01, yangoon wrote: > On 2010年10月25日 14:09:42, ced wrote: > > I find this code is not clear for tutorial > > I can elaborate, that here the path to trytond is set by other means than > environment. As we said above that trytond must be in the PYTHONPATH, it is not required to have this code. And moreover it can confuse reader with unnecessary trick. http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:55: >>> CONFIG['db_type'] = 'sqlite' Why changing configuration from trytond.conf ? It is confusing. http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:73: >>> USER = 1 On 2010年10月25日 21:38:01, yangoon wrote: > On 2010年10月25日 14:09:42, ced wrote: > > I will not use uppercase here > > http://hg.tryton.org/trytond/file/5749fbc08672/trytond/tests/test_tryton.py#l44 Yes because it is a global variable used by others modules. Here we are in an example and using uppercase doesn't help the readability. http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:102: ... cursor.commit() On 2010年10月25日 21:38:01, yangoon wrote: > On 2010年10月25日 14:09:42, ced wrote: > > Why do you commit > > http://hg.tryton.org/trytond/file/5749fbc08672/trytond/tests/test_tryton.py#l123 Perhaps it is not required. http://codereview.appspot.com/2552041/diff/6001/doc/topics/trytondasmodule.rs... doc/topics/trytondasmodule.rst:118: 1 On 2010年10月25日 21:38:01, yangoon wrote: > On 2010年10月25日 14:09:42, ced wrote: > > This test is not reliable > > It is a simple check on the expected result. This doc is not a test. But someone could run it and get an error message which is odd. Why not create a category and add the party on it and then browse it to check if the category is there.