Skip to content

[C API] Add PySys_GetAttr() function #129367

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
vstinner opened this issue Jan 27, 2025 · 4 comments
Closed

[C API] Add PySys_GetAttr() function #129367

vstinner opened this issue Jan 27, 2025 · 4 comments
Labels
topic-C-API type-feature A feature request or enhancement

Comments

@vstinner
Copy link
Member

vstinner commented Jan 27, 2025

Feature or enhancement

Proposal:

The existing PySys_GetObject() function has two issues:

  • It returns a borrowed reference.
  • It ignores errors.

I propose adding new functions PySys_GetAttr() and PySys_GetAttrString() to get a sys module attribute which return a strong reference and don't ignore errors.

API:

PyObject *PySys_GetAttr(PyObject *name)
PyObject *PySys_GetAttrString(const char *name)
  • Return a new object (strong reference) on success.

  • Set an exception and return NULL on error:

    • Set an AttributeError if the attribute doesn't exist.
    • Set a RuntimeError if the sys module cannot be retrieved.

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

No response

Linked PRs

@vstinner
Copy link
Member Author

vstinner commented Jan 27, 2025

An alternative would be to accept an object and add a String function for the const char* argument:

PyObject *PySys_GetAttr(PyObject *name)
PyObject *PySys_GetAttrString(const char *name)

I'm not sure if it's worth it to have an alternative with PyObject*.

UPDATE: I added PySys_GetAttrString(). @serhiy-storchaka conducted a code analysis in 2023 and found that we need a function which takes a PyObject* name.

By the way, I'm also proposing to add PyImport_ImportModuleAttrString(mod_name, attr_name) which is similar: PyImport_ImportModuleAttrString("sys", "path") can be used to get sys.path for example. See issue #128911.

IMO it's still relevant to add PySys_GetAttrString(name) even if PyImport_ImportModuleAttrString(mod_name, attr_name) is added: PySys_GetAttrString(name) is faster, it doesn't have to go into the importlib machinery.

@vstinner
Copy link
Member Author

See also #108512 "C API: Add a replacement for PySys_GetObject".

@serhiy-storchaka
Copy link
Member

Isn't it a duplicate of #108512? That issue turned out to be more complex, because we have at least 4 different functions that implements this with different particularities.

@vstinner
Copy link
Member Author

Isn't it a duplicate of #108512?

Right. I close my issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-C-API type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

2 participants