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

Different behavior between generated model and dynamic loaded #119

Open
AnNeub opened this issue Jan 29, 2022 · 6 comments
Open

Different behavior between generated model and dynamic loaded #119

AnNeub opened this issue Jan 29, 2022 · 6 comments

Comments

@AnNeub
Copy link

AnNeub commented Jan 29, 2022

I was working with a generated model so far. By refactoring my code to also work with dynamic loaded models I run into this issue.

The generator generates for features with upperBound != 1 something like this

    def __init__(self, *, name=None, messages=None):
        # if kwargs:
        #    raise AttributeError('unexpected arguments: {}'.format(kwargs))

        super().__init__(name=name, messages=messages)
 
        if name is not None:
            self.name = name

       if messages is not None:
           self.messages.extend(messages)   #<< relevant part

if you intialize a dynamic loaded class the initialization looks like this:

def new_init(self, *args, **kwargs):
        for name, value in kwargs.items():
            setattr(self, name, value)

which will not work if you provide a list as argument.

I change it to this to get it running.

def new_init(self, *args, **kwargs):
        for name, value in kwargs.items():
            feature = self.eClass.findEStructuralFeature(name)
            if feature:
                if feature.upperBound == 1:
                    setattr(self, name, value)
                else:
                    attr = getattr(self, name)
                    attr.extend(value)
            else:
                raise AttributeError(f'{self.eClass.name} has no attribute "{name}"')

I dont know if there are some drawbacks i have missed. From my point of view both implementation (generated and dynamic) shall behave similar to make it easier to switch between dynamic and generated.

Best regards,
Andreas

@aranega
Copy link
Member

aranega commented Jan 30, 2022

Hi @AnNeub

Thanks so much once again for the issue :) ! You are definitely right, the automatique handling of keyword parameters should be equivalent to the generated code. This is definitely a "half-baked" feature for dynamic metamodels that I didn't finish properly 😅. Your patch is just perfect and gives the same semantic than the generated code. You can use feature.many or feature._many (cached property) to gain a faster access to if yes or not the feature is a collection.

def new_init(self, *args, **kwargs):
    for name, value in kwargs.items():
        feature = self.eClass.findEStructuralFeature(name)
        if feature:
            if feature._many:
                attr = getattr(self, name)
                attr.extend(value)
            else:
                setattr(self, name, value)
        else:
            raise AttributeError(f'{self.eClass.name} has no attribute "{name}"')

I will integrate your patch on develop as soon as possible and add some tests.

Thanks so much again !
Vince

@AnNeub
Copy link
Author

AnNeub commented Jan 30, 2022

Hi Vince,

I tested your implementation and _many does not exists. To be honest I don't check your develop branch if this an improvement there. But manywork so far.
For my patch I use the code generator template as base. So you can think about to use there the same implementation to gain the performance if possible.

This is the macro I am talking about.

{%- macro generate_feature_init(feature) %}
    {%- if feature.upperBound == 1 %}
        if {{ feature.name }} is not None:
            self.{{ feature.name }} = {{ feature.name }}
    {%- else %}
        if {{ feature.name }}:
            self.{{ feature.name }}.extend({{ feature.name }})
    {%- endif %}
{%- endmacro %}

Best regards,
Andreas

@aranega
Copy link
Member

aranega commented Jan 30, 2022

Hi @AnNeub

My bad, the attribute is _many_cache, I forgot I changed that few version ago, but using many will work exactly the same and at reviewing the code I wrote again, using many instead of _many_cache should not degrade performances.

The template you use is just fine as guide. I will change it for a comprehension point of view, but for performances, in the case of the code generator, it will not change anything as the many feature will be accessed only during generation and never handled after.

For dynamic EClass a good speed up for the __init__ method would be to generate/compile the code at run-time and to install it in the EClass (as you suggested). The issue with that is that to keep in sync the new __init__ method with each new EStructuralFeature that could be injected, it would mean that an observer is registerd on each EClass trigerring a new __init__ method compilation every time a new EStructuralFeature is added. This means that during the deserialization of .ecore files, the generation/recompilation of the method would be triggered so often that it will reduce performances.

Perhaps I should had a special method that would trigger the __init__ generation on demand? Leaving the code with your patch as normal behavior and trigerring the generation/recompilation only in case the user wants it? In any case, it will keep the same behavior between dynamic and static.

@AnNeub
Copy link
Author

AnNeub commented Jan 31, 2022

Perhaps I should had a special method that would trigger the init generation on demand? Leaving the code with your patch as normal behavior and trigerring the generation/recompilation only in case the user wants it?
Do you mean if you add structureFeatures during runtime? But will this really need to reinitialize the instance class. I think I don't get your point.

@AnNeub
Copy link
Author

AnNeub commented Feb 4, 2022

One additional thing came up. The init doesn't call the init function of the supertypes. Only the structuralFeatures are overtaken.

I want to replace the init function of my root element to generate some uuid. And so i figured out that is function will be never called.

So far I didn't managed to call the supertypes. I always end up in a recursive loop.

@aranega
Copy link
Member

aranega commented Feb 7, 2022

Hi @AnNeub ;

Arrrg, I think it's related to a problem in the way I'm handling keywords with the notification system. Also, I just saw your issue on pyecore/pyecoregen#31 , perhaps there is a connection here. I'll dig that in details asap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants