Add _mapping property to the result set interface.

[laukhin]

Closes #445

As discussed in the main issue i've added _mapping property to the result set interface. Both asyncpg Record and sqlalchemy Row have this property so it's safe to add it.

[collerek]

This will still be breaking comparing to 0.5.3 due to the fact that Record inherits from Sequence. So it should go into 0.6.0 anyway.

I'm not a big fan of exposing a private property as it's against PEP8 and will annoy a lot of IDEs and linters, but I assume we want to be in line with SQLAlchemy as of now.

[aminalaee]

Should we document the _mapping property too?

I also don't think documenting the private property would be ideal, but I see the initial issue was about this property being unknown.

[collerek]

Should we document the _mapping property too?

I also don't think documenting the private property would be ideal, but I see the initial issue was about this property being unknown.

I believe we should document this property as this will be the only way to get a Mapping.

We can point to mentioned SA docs to emphasise that we wan't to keep it in line with SA.
That way you can process both sqlalchemy Rows and databases Records from raw queries with the same function without any instance checks.

[collerek]

BTW we should probably release a 0.5.5 release reverting the change to keep the 0.5.X line non-breaking so that people with dependencies pinned for i.e. <0.6.0 line won't be affected.

[aminalaee]

Agreed. You mean we should revert #436 for 0.5.5, or something else?

[collerek]

Agreed. You mean we should revert #436 for 0.5.5, or something else?

What I mean is to create a new branch, revert #436 and #408 and publish as 0.5.5 from that branch.
That way we avoid reverting and committing back on master as the next release will be 0.6.0 anyway.
That way we can also fix 0.5 line while we can calmy work on this PR etc.

Branch with only reverted changes:
https://github.com/encode/databases/compare/rel-0-5-5

Draft of the release:
https://github.com/encode/databases/releases/tag/untagged-9fd3d0976bd631adfc23

@aminalaee can you please review this branch changes as there will be no PR to master to avoid creating a mess

[aminalaee]

@collerek That looks good to me.

The reason I asked about the change was that I wanted to say you don't need the documentation changes reverted, as for example database = Database('sqlite+aiosqlite:///example.db') is still correct and the same as database = Database('sqlite:///example.db'), in any version. But anyway you've done that and it looks good.

Not sure how you've tested this, but It might be worth creating a temporary PR for it to make sure all checks will pass and close it later.

[laukhin]

@collerek @aminalaee Hi!
Is adding _mapping property to the documentation enough for this PR to be merged, or I'm missing something?

[collerek]

@collerek @aminalaee Hi! Is adding _mapping property to the documentation enough for this PR to be merged, or I'm missing something?

Yep, missing docs is all we need :)

[laukhin]

Done! I've double checked everything but i'm not a native speaker so feel free to correct any grammar mistakes :)

[aminalaee]

Looks good to me. Thank you.
I'll leave it for final review.

[josheppinette]

EDIT

Okay, I see that this hasn't been released yet, so I am on the old code. I am assuming that this will fix my issue. Are we going to bump a version for this?

---

This is causing mypy to complain:

record = await db.fetch_one(users.select().where(users.c.email == email))
return User.parse_obj(record._mapping) # "Mapping[Any, Any]" has no attribute "_mapping"

I am using the syntax above due to the deprecation warning raised here (which mypy doesn't complain about):

record = await db.fetch_one(users.select().where(users.c.email == email))
return User.parse_obj(record) # DeprecationWarning: The `Row.keys()` method is deprecated
                              # to mimic SQLAlchemy behaviour, use `Row._mapping.keys()` instead.

@laukhin @collerek

[collerek]

Yes, this will be part of the 0.6.0 release (#444)

[circlingthesun]

At least with asyncpg, record['myjson'] is a dict while record._mapping['myjson'] is a str. Is this expected behaviour? I don't really want to manually parse json. If find the Sequence type rather unfortunate because record['myjson'] still works, but now mypy is moaning at me.

[theprestigedog]

Also seeing the same issue as @circlingthesun.
_mapping renders string representations of objects that would previously been set to dict.

[circlingthesun]

@theprestigedog I've created a PR (#501) that restores string indexing on Record objects. It's still pending review.

[mecampbellsoup]

Yes, this will be part of the 0.6.0 release (#444)

Still seeing this issue even on 0.7.0 of this project. We have thousands of emitted warnings:

[cloud-app] tests/gql/subscriptions/test_storage_data.py: 9 warnings
[cloud-app] tests/webhooks/test_chargify_webhook.py: 57 warnings
[cloud-app] tests/webhooks/test_sift_webhook.py: 51 warnings
[cloud-app] tests/gql/subscriptions/test_resources.py: 35 warnings
[cloud-app]   /usr/local/lib/python3.10/site-packages/databases/backends/postgres.py:118: DeprecationWarning: The `Row.keys()` method is deprecated to mimic SQLAlchemy behaviour, use `Row._mapping.keys()` instead.
[cloud-app]     warnings.warn(
[cloud-app]
[cloud-app] tests/test_access_tokens.py::test_gen_random_text
[cloud-app] tests/test_access_tokens.py::test_gen_random_text
[cloud-app]   /usr/local/lib/python3.10/site-packages/databases/backends/postgres.py:128: DeprecationWarning: The `Row.values()` method is deprecated to mimic SQLAlchemy behaviour, use `Row._mapping.values()` instead.
[cloud-app]     warnings.warn(

[pietdaniel]

Also seeing this in the 0.7.0 release.

.../venv/lib/python3.10/site-packages/databases/backends/postgres.py:118: DeprecationWarning: The `Row.keys()`
method is deprecated to mimic SQLAlchemy behaviour, use `Row._mapping.keys()` instead.
    warnings.warn(

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

In [1]: import databases

In [2]: databases.__version__
Out[2]: '0.7.0'