search

wireservice / agate-sql

agate-sql adds SQL read/write support to agate.
https://agate-sql.readthedocs.io
MIT License
18 stars 15 forks source link

Idea to refactor number code #45

Closed clach04 closed 9 months ago

clach04 commented 9 months ago

Related to:

  1. comment https://github.com/wireservice/agate-sql/pull/36#issuecomment-1056033628
  2. change / commit https://github.com/wireservice/agate-sql/commit/4f8ad30a907eb6595ea93874063b50b0362aec81

Idea / Proposal / Question

https://github.com/wireservice/agate-sql/blob/d2bd282c15b25032d7a7c60332cb7743092c461f/agatesql/table.py#L215

Change the include list check to an exclude list. Looking at the code, sounds like PostgreSQL and SQLite could be an exclude list, and maybe easier to maintain.

            if isinstance(column.data_type, agate.Number) and dialect not in ('postgresql', 'sqlite'):
jpmckinney commented 9 months ago

As far as maintenance goes, it’s the same to me whether we include or exclude. What’s important is to document the precision ranges, so that we set a valid default.