[rfile] Add pythonizations #20167
[rfile] Add pythonizations #20167
Conversation
silverweed
requested review from
dpiparo,
pcanal and
vepadulano
as code owners
bindings/pyroot/pythonizations/python/ROOT/_pythonization/_rfile.py
Outdated
Show resolved
Hide resolved
| def rfile_open_wrapper(klass, *args): | ||
| rfile = original(*args) | ||
| rfile._OriginalGet = rfile.Get | ||
| rfile.Get = _RFile_Get(rfile) | ||
| rfile._OriginalPut = rfile.Put | ||
| rfile.Put = _RFile_Put(rfile) | ||
| return rfile |
There was a problem hiding this comment.
It's slightly weird to see these lines here as they do not relate to the Open method per se. Usually these are placed in the function that is wrapped by the @pythonization decorator
| """ | ||
| Prevent the creation of RFile through constructor (must use a factory method) | ||
| """ | ||
| raise NotImplementedError("RFile can only be created via Recreate, Open or Update") |
bindings/pyroot/pythonizations/python/ROOT/_pythonization/_rfile.py
Outdated
Show resolved
Hide resolved
Test Results 20 files 20 suites 3d 13h 13m 37s ⏱️ For more details on these failures, see this check. Results for commit 013c747. ♻️ This comment has been updated with latest results. |
| [[nodiscard]] void *GetUntyped(std::string_view path, | ||
| std::variant<const char *, std::reference_wrapper<const std::type_info>> type) const; |
There was a problem hiding this comment.
Since it is private and only use in the source file, would this be an option:
| [[nodiscard]] void *GetUntyped(std::string_view path, | |
| std::variant<const char *, std::reference_wrapper<const std::type_info>> type) const; | |
| template <typename T> | |
| [[nodiscard]] void *GetUntyped(std::string_view path, | |
| T type) const; |
(TClass::GetClass itself has the 2 overloads).
There was a problem hiding this comment.
I think this can't work because it's also called from the header (in Get,which cannot be moved to the cxx)
There was a problem hiding this comment.
Fair enough. This is then down to a stylistic choice. You could still use the template but with one level of indirection (2 non templated one line overloads implemented in the source file and 1 templated Impl function)
io/io/inc/ROOT/RFile.hxx
Outdated
|
|
||
| /// Returns an **owning** pointer to the object referenced by `key`. The caller must delete this pointer. | ||
| /// This method is meant to only be used by the pythonization. | ||
| [[nodiscard]] void *GetRFileObjectFromKey(RFile &file, const RKeyInfo &key); |
There was a problem hiding this comment.
I think I would prefer a special handling of Get<void>(...) that doesn't check types.
There was a problem hiding this comment.
I'm not sure if you mean:
- in this function (but since it's not templated I'm not sure what you mean)
- instead of this function
- in
RFile::Getin general, unrelated to this function
4 participants
This Pull request:
RFile::GetKeyInfo(necessary for the non-templatedGetpythonization)Get,Put,GetKeyInfoandListKeys.Get/Put: allow them to be called without template arguments, check the type at runtime
GetKeyInfo: return None instead of std::nullopt
ListKeys: use keyword arguments instead of bitmask for flags
Checklist: