pudo / dataset

Easy-to-use data handling for SQL data stores with support for implicit table creation, bulk loading, and transactions.
https://dataset.readthedocs.org/
MIT License
4.76k stars 297 forks source link

Added support for many operators #354

Closed kapily closed 3 years ago

kapily commented 3 years ago

Added support for more operators including:

Instead of returning false when an operator is not found, it will first check to see if SQLAlchemy has an operator by that name. This can reduce a lot of existing redundant code in if statements.

pudo commented 3 years ago

Doesn't this implicitly create quite a number of weird operators, like __len__ or mro?

In any case, I really feel we must fully document the operators thing before we even consider extending it - it's currently totally undocumented, which leads to confusion like in https://github.com/pudo/dataset/issues/353.

kapily commented 3 years ago

Since you have a dependency on SQLAlchemy I think it's ok to mirror the name of the operator in SQLAlchemy? If not, can I just add in the notlike and notilike operators in the if loop?

pudo commented 3 years ago

I'm not so much for or against doing the getattr, I just think we should document the whole query syntax in the docs. I feel this might get a bit harder with the getattr, but not impossible. Would you maybe be up for adding a section to docs/api.rst or a new module?

kapily commented 3 years ago

Do we need to refactor this into a new module - it seems like this is a reasonable place to put it? Also since this is a thin wrapper above SQLAlchemy, I thought it was ok to just transparently support everything that SQLAlchemy does.

I'm also not sure how to add it to the api.rst since this is not a member function but instead just an addition of an if statement.

Is it ok if we merge the PR as-is?

pudo commented 3 years ago

Closing because it's undocumented and would introduce all kinds of unintended behaviour. Sorry, but I'm just not willing to make this arbitrarily complex without clear documentation.