python_require_extend_class method

[memsharded]

Changelog: Feature: Add a new init() method to conanfile.py recipes that can be used to add extra logic when inheriting from python_requires classes.
Docs: conan-io/docs#1622

Close #6612

[jgsogo]

I've tried this, the test passes 😎

from conans import ConanFile

class PkgTest(ConanFile):
    license = "MIT"
    author = "frodo"
    settings = "arch", # tuple!
    python_requires = "base/1.1@user/testing"
    python_requires_extend = "base.MyConanfileBase"
    
    #@classmethod
    #def python_require_extend_class(cls, c):
    #    cls.settings = c.settings + cls.settings

    def __init__(self, *args, **kwargs):
        self.settings = super(PkgTest, self).settings + self.settings
        super(PkgTest, self).__init__(*args, **kwargs)

    def build(self):
        self.output.info("License! %s" % self.license)
        self.output.info("Author! %s" % self.author)
        self.output.info("os: %s arch: %s"
                            % (self.settings.get_safe("os"), self.settings.arch))

[memsharded]

Perfect, much better!

@DoDoENT please try that approach, this can be used today.

@jgsogo please update the PR, I think having the test is still worth it, just in case.

[DoDoENT]

@DoDoENT please try that approach, this can be used today.

I'll try that, but it looks hacky. However, if it works, it could be the basis for implementation for your proposal with python_require_extend_class method (it would get invoked in the default __init__.

[memsharded]

I'll try that, but it looks hacky.

More hacky than a custom method magically invoked by Conan? The syntax might be a bit unknown for some users, but it is quite a pythonic idiom, isn't it?

[DoDoENT]

The syntax might be a bit unknown for some users, but it is quite a pythonic idiom, isn't it?

I wouldn't know - I'm a c++ developer 😸 .

The part that bothers me (although it's probably because I'm not so much into python) is this:

self.settings = super(PkgTest, self).settings + self.settings

It looks like concatenation of settings tuple from the PkgTest and PkgTest's base. And PkgTest's base is ConanFile, so at the first look, it appears that all classes will get the same tuple from ConanFile. It's not evident that "somewhere behind the scenes" something changes the base class from ConanFile to something obtained from python_requires.

Also, when does the change happen? Am I allowed to call super(PkgTest, self).__init__(*args, **kwargs) before manipulation of settings and options or not?

With custom method magically invoked by Conan, at least I am confident that Conan will invoke it at the right moment and any further internal changes to conan will reduce the chance of breaking the expected behaviour.

[memsharded]

It looks like concatenation of settings tuple from the PkgTest and PkgTest's base. And PkgTest's base is ConanFile, so at the first look, it appears that all classes will get the same tuple from ConanFile. It's not evident that "somewhere behind the scenes" something changes the base class from ConanFile to something obtained from python_requires.

I see the point, let's have a second thought.

[danimtb]

I think this is something quite pythonic and I like it. However, it would be better to have all the attributes defined in the same place (all of them in the __init__ for example), but I think it is not possible.

Nevertheless, I see the syntax makes sense and if people are using python_requires to do any kind of inheritance or pure python code reuse, I believe they should understand how python works for this matter.

Additionally, we would have to clearly define the usage and provide meaningful examples in the documentation.

[memsharded]

if people are using python_requires to do any kind of inheritance or pure python code reuse, I believe they should understand how python works for this matter.

That is the thing, that we are hiding the inheritance, making it lazy. So it is not evident where are you inheriting from. Otherwise, I'd agree that the solution is perfect.

[DoDoENT]

I've tried this:

class MyPackage(ConanFile):
    python_requires = 'MyBaseConanFile/1.0.0@company/stable'
    python_requires_extend = "MyBaseConanFile.MyBaseConanFile"
 
    options = {
        'another_option': [True, False]
    }

    def __init__(self, *args, **kwargs):
        super(MyPackage, self).__init__(*args, **kwargs)
        self.options.update(super(MyPackage,self).options)

and it appears to work.

If you guys plan to keep this pattern stable for general usage, I'll be OK with using it. However, I am afraid that it will be a bit more complex to explain to my developers what is happening here. The issue is that it's not clear at which point does the class extending happens (obviously before invoking the constructor). Also, since now python_requires dependencies are also used for calculation of package_id, could it happen that options/settings inherited from the base will not take part of the packageID calculation (as those class attributes are injected into the class after it has been constructed)? This boggles my mind quite a bit, but it could be just because I am used to the way of thinking in the static, type-safe, C++ world.

In general, if that would be the official pattern for inheriting options and settings from the base conanfile, please document it here (currently python_requires_extend is not documented in the attribute reference at all - only the python_requires is).

Also, one question: since the new python_requires is still in the experimental phase, what are your future plans on breaking it/keeping it stable? I am asking this because I am currently quite hesitant to migrate 150+ recipes to the new syntax if it's likely that it's going to break in the future.

[DoDoENT]

Also, just out of curiosity, how did you plan to support multiple python requirements and multiple inheritances with the new model? I didn't need that so far, but in the old model it was theoretically achievable: the python_requires was just a python function which I could call as many times as I wanted to obtain as many "base" objects as I wished and in theory, I could inherit my class from all of them. Note that I didn't try if that works. I guess there could be problems if more of them already inherit ConanFile, but with this approach I could mix-in as many classes as I wish.

[memsharded]

Yes, that is a good point. The def python_require_extend_class(cls, c): allows handling multiple inheritance more intuitively.

[DoDoENT]

Yes, that is a good point. The def python_require_extend_class(cls, c): allows handling multiple inheritance more intuitively.

Actually, we would need a syntax to tell multiple python requirements (i.e. python_requires will have to be a tuple, just like requires is), python_requires_extend will have to be a tuple and the signature of python_require_extend_class would have to allow for accessing of multiple base classes (probably the c parameter would be a tuple in case of multiple inheritance).

I guess the same could be achieved also with hacked __init__ method, but somehow there will be a need for referencing the base class you want to obtain settings/options from.

[jgsogo]

Another approach I'd really like more would be to use the initialize function (in the __init__ one it works too). Below we are not using the inheritance concept, we are just getting some value from the python_requires (.module.MyConanfileBase.settings) and using it before Conan uses the settings and options:

from conans import ConanFile
class PkgTest(ConanFile):
    license = "MIT"
    author = "frodo"
    settings = "arch", # tuple!
    python_requires = "base/1.1@user/testing"
    python_requires_extend = "base.MyConanfileBase"
    
    def initialize(self, *args, **kwargs):
        self.settings = self.python_requires['base'].module.MyConanfileBase.settings + self.settings
        self.license = self.python_requires['base'].module.MyConanfileBase.license
        self.author = self.python_requires['base'].module.MyConanfileBase.author
        super(PkgTest, self).initialize(*args, **kwargs)

    def build(self):
        self.output.info("License! %s" % self.license)
        self.output.info("Author! %s" % self.author)
        self.output.info("os: %s arch: %s" % (self.settings.get_safe("os"), self.settings.arch))

This is just another approach to consider.

[memsharded]

Furthermore, we have full control, we could use another method, not a class one, and call it from conan, avoiding all super() calls completely and not messing with the current initialize() that so far is an internal implementation detail. Probably the access via self.python_requires is better, but I think I would use a dedicated method. Lets see what other say.

[DoDoENT]

Well, from my perspective, I am OK with both __init__, initialize, python_require_extend_class or anything else, as long as its stable across different versions of conan (i.e. that it isn't hacking into some private implementation detail that may change at any time).

An additional benefit would be to be able to easily understand the process of extending so the training of developers would be easier.

[DoDoENT]

Also, accessing via self.python_requires['base'].module.MyConanfileBase is consistent with accessing other module-level definitions in the base, so I am OK with that.

But, @jgsogo, keep in mind how multiple inheritance would work. For accessing the base, the self.python_requires seems OK, but how to specify multiple inheritance with current syntax of python_requires_extend? Also, how to have multiple python requirements?

[memsharded]

Please check the latest changes with a extend() regular method proposal. (name of the method to be changed, proposals welcome). I think the implementation is quite simple and probably the easiest to understand by the users, no ambiguity there. The only caveat is the potential this method has to mess and programatically define other things rather than what the method was intended for, as it always happen with every new feature... 😅

[jgsogo]

I prefer to call the function initialize (we can move the internal one to a different name), but yes, with one of these functions it is the way to go. Let's open Pandora's box, once again, let's see what people come up with.

But, @jgsogo, keep in mind how multiple inheritance would work. For accessing the base, the self.python_requires seems OK, but how to specify multiple inheritance with current syntax of python_requires_extend? Also, how to have multiple python requirements?

I think that this test contains what you are looking for:

conan/conans/test/functional/py_requires/python_requires_test.py

Line 121 in 4bfe9a6

class MyConanfileBase(ConanFile):

[DoDoENT]

I agree with @jgsogo - I think the initialize is the better name, as it suggests it's the very first thing that will be called (even before config_options). That is not so clear with extend, which, on the other hand, better suggests its intention.

What about the original suggestion by @memsharded: python_require_extend_class, but this time as a regular method, not a class method? Its name suggests a clear intention - do stuff here that is influenced by python requirements, such as merging of attributes and similar.

[jgsogo]

IMO this initialize/extend function is not only related to python_requires, but I'm also sure people will use it for the logic inside the set_name and set_version functions too, for example, or even for some logic related to scm feature. And, naming it xxx_python_requires_xxx won't discourage people to use it for other things.

[memsharded]

Renamed it to init() to avoid possible side effects, breaking users that (illegally) are using the initialize() method.

I would document that explicitly for this purpose only, and we will see. The truth is that if it is useful for other things then later it is very ugly to be abusing a python-requires-extend() method for other unrelated purposes. Lets open the pandora box :)

[jgsogo]

Need changlog and docs