Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(548)

Issue 322730043: trytond-gis: Initial commit

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 months, 1 week ago by nicoe
Modified:
12 hours, 42 minutes ago
Reviewers:
ced
Visibility:
Public.

Patch Set 1 #

Total comments: 12

Patch Set 2 : Fix comments and move regexp #

Patch Set 3 : rename abstract_type and move regexp to prevent circular imports #

Patch Set 4 : Add forgotten database.py #

Patch Set 5 : Add forgotten database.py #

Total comments: 21

Patch Set 6 : Use setUpModule to populate the module list and register the testing module #

Unified diffs Side-by-side diffs Delta from patch set Stats (+978 lines, --1 lines) Patch
A COPYRIGHT View 1 chunk +15 lines, -0 lines 0 comments Download
A LICENSE View 1 chunk +674 lines, -0 lines 0 comments Download
A MANIFEST.in View 1 chunk +4 lines, -0 lines 0 comments Download
A README View 1 chunk +8 lines, -0 lines 0 comments Download
A setup.py View 1 2 3 4 5 1 chunk +52 lines, -0 lines 0 comments Download
A tox.ini View 1 2 3 4 5 1 chunk +13 lines, -0 lines 0 comments Download
A trytond_backend_gis/__init__.py View 1 2 3 4 5 1 chunk +3 lines, -0 lines 0 comments Download
A trytond_backend_gis/fields.py View 1 2 3 4 5 1 chunk +49 lines, -0 lines 0 comments Download
A trytond_backend_gis/postgis/__init__.py View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
A trytond_backend_gis/postgis/database.py View 1 2 3 4 5 1 chunk +19 lines, -0 lines 0 comments Download
A trytond_backend_gis/postgis/table.py View 1 2 3 4 5 1 chunk +39 lines, -0 lines 0 comments Download
A trytond_backend_gis/tests/__init__.py View 1 2 3 4 5 0 chunks +-1 lines, --1 lines 0 comments Download
A trytond_backend_gis/tests/test_fields.py View 1 2 3 4 5 1 chunk +71 lines, -0 lines 0 comments Download
A trytond_backend_gis/tests/test_gis/__init__.py View 1 2 3 4 5 1 chunk +12 lines, -0 lines 0 comments Download
A trytond_backend_gis/tests/test_gis/gis.py View 1 2 3 4 5 1 chunk +14 lines, -0 lines 0 comments Download
A trytond_backend_gis/tests/test_gis/tryton.cfg View 1 2 3 4 5 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 11
nicoe
3 months, 1 week ago (2017-03-17 13:53:08 UTC) #1
ced
https://codereview.appspot.com/322730043/diff/1/setup.py File setup.py (right): https://codereview.appspot.com/322730043/diff/1/setup.py#newcode10 setup.py:10: setup(name='trytond_gis', Maybe better: trytond_backend_gis https://codereview.appspot.com/322730043/diff/1/setup.py#newcode14 setup.py:14: url='http://trytond-gis.b2ck.com/', I'm wondering ...
3 months, 1 week ago (2017-03-17 14:36:10 UTC) #2
nicoe
https://codereview.appspot.com/322730043/diff/1/setup.py File setup.py (right): https://codereview.appspot.com/322730043/diff/1/setup.py#newcode10 setup.py:10: setup(name='trytond_gis', On 2017/03/17 14:36:10, ced wrote: > Maybe better: ...
3 months, 1 week ago (2017-03-20 17:19:42 UTC) #3
nicoe
Fix comments and move regexp
3 months, 1 week ago (2017-03-20 17:20:11 UTC) #4
nicoe
rename abstract_type and move regexp to prevent circular imports
3 months ago (2017-03-23 16:25:36 UTC) #5
nicoe
Add forgotten database.py
3 months ago (2017-03-23 16:41:51 UTC) #6
ced
https://codereview.appspot.com/322730043/diff/80001/setup.py File setup.py (right): https://codereview.appspot.com/322730043/diff/80001/setup.py#newcode16 setup.py:16: description='Adds GIS support to trytond', Maybe use: Geographical Information ...
1 month, 1 week ago (2017-05-19 16:23:25 UTC) #7
ced
I'm wondering if we should not use https://github.com/cleder/pygeoif as suggested by https://stackoverflow.com/questions/14940285/using-postgis-on-python-3
1 month, 1 week ago (2017-05-19 16:36:23 UTC) #8
nicoe
Use setUpModule to populate the module list and register the testing module
12 hours, 42 minutes ago (2017-06-25 19:57:46 UTC) #9
nicoe
On 2017/05/19 16:36:23, ced wrote: > I'm wondering if we should not use https://github.com/cleder/pygeoif > ...
12 hours, 42 minutes ago (2017-06-25 19:58:27 UTC) #10
nicoe
12 hours, 42 minutes ago (2017-06-25 19:58:37 UTC) #11
https://codereview.appspot.com/322730043/diff/80001/setup.py
File setup.py (right):

https://codereview.appspot.com/322730043/diff/80001/setup.py#newcode16
setup.py:16: description='Adds GIS support to trytond',
On 2017/05/19 16:23:24, ced wrote:
> Maybe use: Geographical Information System

Done.

https://codereview.appspot.com/322730043/diff/80001/setup.py#newcode27
setup.py:27: 'Programming Language :: Python :: 2',
On 2017/05/19 16:23:24, ced wrote:
> I do not know what this means? Which version?

Done.

https://codereview.appspot.com/322730043/diff/80001/trytond_backend_gis/__ini...
File trytond_backend_gis/__init__.py (right):

https://codereview.appspot.com/322730043/diff/80001/trytond_backend_gis/__ini...
trytond_backend_gis/__init__.py:3: # terms.
On 2017/05/19 16:23:24, ced wrote:
> Usually we have only 2 lines.

Usually module names are not that long.

https://codereview.appspot.com/322730043/diff/80001/trytond_backend_gis/field...
File trytond_backend_gis/fields.py (right):

https://codereview.appspot.com/322730043/diff/80001/trytond_backend_gis/field...
trytond_backend_gis/fields.py:13: if 'loading' not in kwargs:
On 2017/05/19 16:23:24, ced wrote:
> Why not having loading defined in __init__ signature?

Done.

https://codereview.appspot.com/322730043/diff/80001/trytond_backend_gis/field...
trytond_backend_gis/fields.py:15: super(Geometry, self).__init__(**kwargs)
On 2017/05/19 16:23:24, ced wrote:
> is it not possible to have: __init__(string=string, loading=loading, **kwargs)

Done.

https://codereview.appspot.com/322730043/diff/80001/trytond_backend_gis/postg...
File trytond_backend_gis/postgis/__init__.py (right):

https://codereview.appspot.com/322730043/diff/80001/trytond_backend_gis/postg...
trytond_backend_gis/postgis/__init__.py:2: DatabaseOperationalError)
On 2017/05/19 16:23:24, ced wrote:
> You could import them in database and have from .database import *

Done.

https://codereview.appspot.com/322730043/diff/80001/trytond_backend_gis/postg...
File trytond_backend_gis/postgis/database.py (right):

https://codereview.appspot.com/322730043/diff/80001/trytond_backend_gis/postg...
trytond_backend_gis/postgis/database.py:15: connection = super(Database,
self).get_connection(autocommit, readonly)
On 2017/05/19 16:23:25, ced wrote:
> Use keyworded values, it is safer in case of change.

Done.

https://codereview.appspot.com/322730043/diff/80001/trytond_backend_gis/postg...
File trytond_backend_gis/postgis/table.py (right):

https://codereview.appspot.com/322730043/diff/80001/trytond_backend_gis/postg...
trytond_backend_gis/postgis/table.py:6: from trytond.backend.postgresql.table
import _NO_DEFAULT
On 2017/05/19 16:23:25, ced wrote:
> Does not exist.

Done.

https://codereview.appspot.com/322730043/diff/80001/trytond_backend_gis/postg...
trytond_backend_gis/postgis/table.py:8: from trytond_backend_gis import GIS_RE
On 2017/05/19 16:23:25, ced wrote:
> Will it not be better to make a relative import?
> 
> Indeed I think it is used only here, so better to move it.

Done.

https://codereview.appspot.com/322730043/diff/80001/trytond_backend_gis/postg...
trytond_backend_gis/postgis/table.py:15: def add_column(self, column_name,
abstract_type, default=_NO_DEFAULT,
On 2017/05/19 16:23:25, ced wrote:
> default is None

Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 80a51fa-tainted