Skip to content
This repository was archived by the owner on Jan 13, 2022. It is now read-only.

Add support for converting numpy integers. #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tandreas
Copy link

Currently, reflecting MetaData using sqlalchemy-monetdb with monetdblite doesn't work.

For example:

from sqlalchemy import create_engine, MetaData, Table

engine = create_engine('monetdb+lite:///:memory', echo=True)
metadata = MetaData(bind=engine)
query = """
CREATE TABLE "test" (
    id int,
    data varchar(128)
)
"""
engine.execute(query)
metadata = MetaData(bind=engine)
tbl = Table('test', metadata, autoload=True)

This happens because when sqlalchemy-monetdb tries to round trip integer ids, they are coming back from monetdblite as numpy.int32, and this isn't supported by monetize.convert.

This adds support for that. I think the fix should be in monetdblite rather than sqlalchemy-monetdb, a user will expect that data types retrieved from the driver should be able to round trip back in a query.

@hannes
Copy link
Contributor

hannes commented Oct 24, 2017

@Mytherin & @gijzelaerr any comments on this?

@Mytherin
Copy link
Collaborator

Not sure if this is the correct way to fix this problem. Assuming the SQLAlchemy driver is only using the regular Python DB interface, I don't think MonetDBLite should be returning numpy integers at all. I haven't looked at the SQLAlchemy MonetDBLite driver, though, so I don't know what is exactly happening there.

@gijzelaerr
Copy link
Collaborator

gijzelaerr commented Oct 25, 2017

I actually don't think this is a bad idea. This way numpy ints are rendered properly inside strings. Probably a good idea to add the other numpy types (like float) also. SQLAlchemy is using the MonetDBLite API, which internally constructs numpy arrays for result sets which contain numpy ints. I guess this is not a problem, as long as numpy ints evaluate equal to python ints.

I'm not a big fun of try catching the numpy import. If you leave it out the interpreter will gave a similar error, which is quite obvious.

@gijzelaerr
Copy link
Collaborator

this is also a problem with booleans, which are returned as 0 and 1 type uint8. this is a bit more worrying.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants