Is this Python decorator a good idea to solve circular recursion?

Question

I have several model classes, let's say A_0, A_1, ..., A_N

I have a method A_0.to_A_1() that calls A_1.to_A_2(), ... that calls ..., that calls A_(N-1).to_A_N()

So far so good, but A_N may then call again to_A_M() with M < N if A_N is an instance of A_K with K < M.

That's bad! So I came up with this neat solution:

from functools import wraps
import threading

def nrec(func):
    thread_local = threading.local()

    @wraps(func)
    def wrapper(*args, **kwargs):
        if getattr(thread_local, '__nrec_visited', False):
            return None
        
        thread_local.__nrec_visited = True
        
        result = func(*args, **kwargs)
        
        thread_local.__nrec_visited = False
        return result
    
    return wrapper

I think this is so cool, also thread-safe so I can use it with FastAPI and such.

Here's an example:

class OrganizationData(SQLModel, table=True):
    __tablename__ = "organization"
    __table_args__ = {"schema": "stackoverflow"}

    organization_id: int | None = Field(default=None, primary_key=True)
    organization_name: str

    services: list["ServiceData"] = Relationship(link_model=OrganizationServiceData)
    workspaces: list["WorkspaceData"] = Relationship(link_model=OrganizationWorkspaceData)

    @nrec
    def to_organization(self):
        return Organization(
            id=self.organization_id,
            name=self.organization_name,
            services=[service.to_service() for service in self.services],
            workspaces=[w for workspace in self.workspaces if (w := workspace.to_workspace())] or None
        )

class Organization(BaseModel):
    id: int | None = None
    name: str
    
    services: list[Service]
    workspaces: list["Workspace"] | None

class WorkspaceData(SQLModel, table=True):
    __tablename__ = "workspace"
    __table_args__ = {"schema": "stackoverflow"}

    workspace_id: int | None = Field(default=None, primary_key=True)
    workspace_name: str
    organization_id: int = Field(foreign_key=".organization.organization_id", ondelete="CASCADE")

    organization: OrganizationData = Relationship()
    services: list["ServiceData"] = Relationship(link_model=WorkspaceServiceData)

    @nrec
    def to_workspace(self):
        return Workspace(
            id=self.workspace_id,
            name=self.workspace_name,
            organization=self.organization.to_organization(),
            services=[service.to_service() for service in self.services]
        )

class Workspace(BaseModel):
    id: int | None
    name: str
    organization: Organization | None
    services: list[Service]

From an organization I want to get the workspaces, and from a workspace I want to get the organization. But of course, I have no interest in having the workspace's organization of an already given organization. Hope it makes sense.

Please tell me if this is the correct solution or if there is a cleaner way to do this.

Perhaps.

ShapeOfMatter · Accepted Answer · 2024-10-19 18:53:41Z

I have mixed feelings about this. My initial instinct is that there are too many things one might mean by "avoid circular recursion" for any off-the-shelf wrapper to be suitable. But having read it a couple times, the function you've written does serve a clear purpose, and I myself have been in situations where it probably would have been useful.

(Is there a cleaner way to do it? I don't know of any uncontentious alternative approaches, although I'm personally inclined toward the explicitness of passing functions through wrappers as-needed instead of decorating them. But there's downsides to that too.)

So code review!

It'd sure be nice to have type hints. I don't actually know how those interact with wrappers; I haven't had to write any python in a few years. But they're always good to have.
At minimum, you want to make sure you're not breaking the typing of the functions you're wrapping. Notice that whatever the original return type was, you've now added the possibility that it could return None. You should check that your wrapper is either enforcing/preserving or updating the type signature of the wrapped function. Probably wraps is already doing something sensible...
Complicating the above point, make the substituted value configurable. Give nrec an extra arg that defaults to None to be used when it interrupts recursion.
Complicating the above point, make the substitution a function. I agree that most of the time you'll use a constant value, but it'd be nice to be able to redirect the arguments of the recursive call to some other handler. For example instead of None, you might want the workspace's ID number.
A better name would be good, e.g. non_recursive.

RootTwo · Accepted Answer · 2024-11-03 04:59:10Z

2

I wonder if you couldn't get the same general effect using functools.cache(). It wouldn't be exactly the same, because a call to a previously visited function would return the previously calculated return value rather than None.

answered Nov 3, 2024 at 4:59

RootTwo

10.7k1 gold badge14 silver badges30 bronze badges

Add a comment |

Stack Exchange Network

Is this Python decorator a good idea to solve circular recursion?

2 Answers 2

You must log in to answer this question.

Hot Network Questions

Is this Python decorator a good idea to solve circular recursion?

2 Answers 2

You must log in to answer this question.

Related

Hot Network Questions