NWChemEx / NWChemEx/PluginPlay

Remove shared_ptr from Python API

Open
#271 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
13
Forks
1
Avg merge
43m
Merged PRs (30d)
4

Description

AFAIK the only way to add a Python module to PluginPlay is via a pattern like:

# Redundant import is issue
from pluginplay import pluginplay
from cppyy.gbl.std import make_shared

class MyModule(pluginplay.ModuleBase):
    def __init__(self):
        super().__init__(self)
        # rest of ctor

    
    def run_(self, inputs, submods):
        # rest of body

mm = pluginplay.ModuleManager()
mm.add_module("module key", make_shared[MyModule]())

# Ideal solution
# (currently doesn't work see #269)
# mm.add_module("module key", MyModule())

#Also fine 
#(currently doesn't work leads to double free or corruption error)
# mm.add_module[MyModule]("module key")

# Alternatively could expose a Python free function to wrap
# calling shared_ptr from Python, e.g.
#pluginplay.add_module(mm, MyModule)
#

Relying on std::shared_ptr is very non-Pythonic and I would suspect users would prefer one of the three suggestions over dealing with std::shared_ptr

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing the Python binding entry points for ModuleBase and ModuleManager.add_module, then read the referenced issue #269 and compare the three proposed calling patterns. Done means a Python module can be registered without users explicitly constructing std::shared_ptr, while avoiding the reported double-free or corruption failure.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, python
Domain
api, developer-experience
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
28/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.