Skip to content
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

How to satisfy unexpected-keyword-arg in derived class override method that creates an instance via a super() method #9758

Open
hepcat72 opened this issue Jun 30, 2024 · 3 comments
Labels
Documentation 📗 Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling Question

Comments

@hepcat72
Copy link

hepcat72 commented Jun 30, 2024

Question

I have a model superclass called MaintainedModel. The latest version of superlinter gives me an unexpected-keyword-arg error about a call to an overridden method inside another overridden method. There are maybe a dozen such calls in the class, but only one triggers the linter error.

I believe I understand why I get the error about the one call, but I don't know how to satisfy the linter. I think it's because the overridden method creates an instance via a super()-method call. The linter, in that context, doesn't know about the keyword args I added in the other overridden method. To (over-)simplify it, it's basically this:

def from_db(self):
    instance = super().from_db()
    instance.save(custom_arg=whatever). # unexpected-keyword-arg

I tried adding a typehint to the instance, but that did not prevent the error.

I have more specifics in the following stack post:

https://stackoverflow.com/questions/78645414/1-out-of-many-calls-to-overridden-django-model-save-generates-a-pylint-unex

The call to save does work and my override of save() gets the custom args. How do I tell pylint that the args are OK?

Documentation for future user

https://pylint.readthedocs.io/en/latest/user_guide/messages/error/unexpected-keyword-arg.html

Additional context

I asked on this stack post: https://stackoverflow.com/questions/78645414/1-out-of-many-calls-to-overridden-django-model-save-generates-a-pylint-unex

and I was just advised to have pylint ignore the call to save, but I'd like to either change the code to do things "properly" or figure out how to correctly typehint the code.

EDIT: Oh, another important piece of info (and maybe this answers my question?) is that my override of save() doesn't declare the arts in the signature. It just pops them off of kwargs before calling super().save(*args, **kwargs). Perhaps adding them to the signature will satisfy it? But none of the other save calls in the class ellicit the error, so I felt like maybe that's not going to fix it?

@hepcat72 hepcat72 added Documentation 📗 Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling Question labels Jun 30, 2024
@DanielNoord
Copy link
Collaborator

Without the actual implementation/code it will be very hard to help you out. Are you able to share it?

@hepcat72
Copy link
Author

hepcat72 commented Jul 3, 2024

The code (minus doc string/comments) where the error line is, is in the stack post, though the save override isn't there. I'll paste it all here...

Unedited method where the error occurs (the line with call to save()):

    @classmethod
    def from_db(cls, *args, **kwargs):
        """
        This is an override of Model.from_db.  Model.from_db takes arguments: db, field_names, values.  It is used to
        convert SQL query results into Model objects.  This over-ride uses this opportunity to perform lazy-auto-updates
        of Model fields.  Note, it will not change the query results in terms of records.  I.e. if the maintained field
        value is stale (e.g. it should be "x", but instead is null, and the query is "WHERE field = 'x'", the record
        will NOT be updated because it would not have been returned by the SQL query.  This lazy-auto-update will occur
        when a QuerySet is iterated over.  That's when `from_db` is called - at each iteration.

        This method checks the field_names for the presence of maintained fields, and if the corresponding value is
        None, it will trigger an auto-update and set the new value for the superclass method's model object creation.

        Note, one down-side of this implementation of lazy auto-updates is that if the auto-update results in the value
        being null/None, the code to auto-update the field will always execute (a waste of effort).

        This will not lazy-update DEFERRED field values.
        """
        # Instantiate the model object
        rec = super().from_db(*args, **kwargs)

        # If autoupdates are not enabled (i.e. we're not in "lazy" mode)
        if not cls.get_coordinator().are_lazy_updates_enabled():
            return rec

        # Get the field names
        queryset_fields = set(args[1])  # field_names

        # Intersect the queryset fields with the maintained fields
        common_fields = set(cls.get_my_update_fields()).intersection(queryset_fields)

        # Look for maintained field values that are None
        lazy_update_fields = [fld for fld in common_fields if getattr(rec, fld) is None]

        # If any maintained fields are to be lazy-updated
        if len(lazy_update_fields) > 0:
            cs = ", "
            print(
                f"Triggering lazy auto-update of fields: {cls.__name__}.{{{cs.join(lazy_update_fields)}}}"
            )
            # Trigger an auto-update
            rec.save(fields_to_autoupdate=lazy_update_fields, via_query=True)

        return rec

The save() override... (I removed the comments, which were a bit cluttery.) Much of this is likely irrelevant to the issue. Probably only the first half dozen lines are useful for this issue. The code is a bit old and is due for a streamline.

    def save(self, *args, **kwargs):
        """
        This is an override of the derived model's save method that is being used here to automatically update
        maintained fields.
        """
        mass_updates = kwargs.pop("mass_updates", False)
        propagate = kwargs.pop("propagate", not mass_updates)
        fields_to_autoupdate = kwargs.pop("fields_to_autoupdate", None)
        via_query = kwargs.pop("via_query", False)

        if self is None:
            self._maintained_model_setup(**kwargs)

        coordinator = self.get_coordinator()

        if not hasattr(self, "super_save_called") or not mass_updates:
            self.super_save_called = False

        if not coordinator.are_autoupdates_enabled():
            self.label_filters = coordinator.default_label_filters
            self.filter_in = coordinator.default_filter_in

            if not mass_updates:
                super().save(*args, **kwargs)
                self.super_save_called = True
                coordinator.buffer_update(self)
                return

        if self.pk is None and mass_updates is False:
            super().save(*args, **kwargs)
            self.super_save_called = True

        if (
            (via_query and not coordinator.are_lazy_updates_enabled())
            or (
                not via_query
                and coordinator.are_lazy_updates_enabled()
                and not coordinator.are_immediate_updates_enabled()
            )
        ):
            if hasattr(self, "super_save_called"):
                delattr(self, "super_save_called")
            return

        changed = self.update_decorated_fields(fields_to_autoupdate)

        if changed is True or self.super_save_called is False:
            if self.super_save_called or mass_updates is True:
                if mass_updates is True:
                    self.exists_in_db(raise_exception=True)
                super().save()
            else:
                super().save(*args, **kwargs)

        delattr(self, "super_save_called")

        if coordinator.are_immediate_updates_enabled() and propagate:
            self.call_dfs_related_updaters()

There are other various calls to save that do not ellicit any linting error, such as:

rec.save(mass_updates=True)
parent_inst.save(propagate=False, mass_updates=mass_updates)
child_inst.save(propagate=False, mass_updates=mass_updates)
buffer_item.save(mass_updates=True)

@hepcat72
Copy link
Author

hepcat72 commented Jul 3, 2024

The entirety of the code is here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation 📗 Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling Question
Projects
None yet
Development

No branches or pull requests

2 participants