Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

bpo-40563: Support pathlike objects on dbm/shelve#20274

Closed
hakancelikdev wants to merge 4 commits into
python:masterpython/cpython:masterfrom
hakancelikdev:bpo-40563Copy head branch name to clipboard
Closed

bpo-40563: Support pathlike objects on dbm/shelve#20274
hakancelikdev wants to merge 4 commits into
python:masterpython/cpython:masterfrom
hakancelikdev:bpo-40563Copy head branch name to clipboard

Conversation

@hakancelikdev

@hakancelikdev hakancelikdev commented May 20, 2020

Copy link
Copy Markdown
Contributor

@remilapeyre remilapeyre left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @hakancelik96, thanks for opening this PR!

There is some parts in dbm that were not updated:

>>> fn = pathlib.Path('foo')
>>> dbm.dumb.open(fn)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/remi/src/cpython/Lib/dbm/dumb.py", line 316, in open
    return _Database(file, mode, flag=flag)
  File "/Users/remi/src/cpython/Lib/dbm/dumb.py", line 57, in __init__
    self._dirfile = filebasename + '.dir'
TypeError: unsupported operand type(s) for +: 'PosixPath' and 'str'
>>> dbm.gnu.open(fn)
Exception ignored in: <function _Database.close at 0x101c69370>
Traceback (most recent call last):
  File "/Users/remi/src/cpython/Lib/dbm/dumb.py", line 274, in close
    self._commit()
  File "/Users/remi/src/cpython/Lib/dbm/dumb.py", line 116, in _commit
    if self._index is None or not self._modified:
AttributeError: '_Database' object has no attribute '_index'
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: open() argument 1 must be str, not PosixPath
>>> dbm.ndbm.open(fn)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: open() argument 1 must be str, not PosixPath

and I think the documentation also needs to be updated, see for example the note added to shutil.move() documentation when it was updated to support pathlib: #15326.

Thanks!

@hakancelikdev

Copy link
Copy Markdown
Contributor Author

@remilapeyre thanks for your review, I will send a new commit when I solve these issues.

@remilapeyre

Copy link
Copy Markdown

Hi @hakancelik96, thanks for updating the rest of the module!

It looks like there is some merge conflicts, could you fix these? I think running make regen-all should do it :)

@remilapeyre remilapeyre left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is some syntax errors, I'm not sure how the CI tests passed with them.

Comment thread Modules/_dbmmodule.c Outdated
Comment thread Modules/_gdbmmodule.c Outdated
@hakancelikdev

Copy link
Copy Markdown
Contributor Author

@remilapeyre Thanks for reviewing. I resolved syntax issues.

@remilapeyre

Copy link
Copy Markdown

Thanks for fixing the syntax issues @hakancelik96, please have a look at the compilation errors:

/Users/remi/src/cpython/Modules/_dbmmodule.c:473:15: error: redefinition of 'filenamebytes'
    PyObject *filenamebytes = PyUnicode_EncodeFSDefault(filename);
              ^
/Users/remi/src/cpython/Modules/_dbmmodule.c:463:15: note: previous definition is here
    PyObject *filenamebytes = PyOS_FSPath(filename);
              ^
1 error generated.
/Users/remi/src/cpython/Modules/_gdbmmodule.c:662:15: error: redefinition of 'filenamebytes'
    PyObject *filenamebytes = PyUnicode_EncodeFSDefault(filename);
              ^
/Users/remi/src/cpython/Modules/_gdbmmodule.c:652:15: note: previous definition is here
    PyObject *filenamebytes = PyOS_FSPath(filename);

@hakancelikdev

Copy link
Copy Markdown
Contributor Author

I'll send a new PR with a clear history

@remilapeyre

Copy link
Copy Markdown

Don't worry too much about the history, everything will be squashed anyway and it's normal to write corrections for a PR.

@audeoudh

Copy link
Copy Markdown
Contributor

I'll send a new PR with a clear history

Any link to that PR? Or can you say if you are not working on it anymore?

@hakancelikdev

Copy link
Copy Markdown
Contributor Author

@audeoudh I am not working on anymore.

@DavidMertz

DavidMertz commented Sep 9, 2021

Copy link
Copy Markdown

I've made the few additional changes to those in this PR. When I work out the issues, I'll make a new PR. I took out an attempt with path_t. However, here is why I think argument clinic (or something else?!) is actually intercepting the attempted call:

With my temporary debugging, I have this function in Modules/_gdbmmodule.c:

[clinic start generated code]*/

static PyObject *
dbmopen_impl(PyObject *module, PyObject *filename, const char *flags,
             int mode)
/*[clinic end generated code: output=9527750f5df90764 input=812b7d74399ceb0e]*/
{
    PyObject_Print(filename, stdout, 0);
    printf(" from _gdbmmodule.c (XXX)\n");
    /* ... rest of function ...*/

And I have a very simplified test script:

import _gdbm
import sys
from pathlib import Path

print(sys.version)
path = '/tmp/tmp.db'

db = _gdbm.open(path, 'c')
print("Opened with string path")
db.close()

db = _gdbm.open(Path(path), 'c')
print("Opened with path-like")
db.close()

The output of running this is:

3.11.0a0 (heads/[bpo-45133](https://bugs.python.org/issue45133)-dirty:0376feb030, Sep  8 2021, 00:39:39) [GCC 10.3.0]
'/tmp/tmp.db' from _gdbmmodule.c (XXX)
Opened with string path
Traceback (most recent call last):
  File "/home/dmertz/tmp/pathlike-dbm.py", line 12, in <module>
    db = _gdbm.open(Path(path), 'c')
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: open() argument 1 must be str, not PosixPath

So before I get to the first line of the _gdbm.open() function, the TypeError is already occurring when passed a PosixPath.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.