Skip to content

Issue 233 cfg property and document parent init call#234

Merged
eldipa merged 27 commits intomasterfrom
Issue-233-Cfg-Property-And-Document-Parent-Init-Call
May 14, 2022
Merged

Issue 233 cfg property and document parent init call#234
eldipa merged 27 commits intomasterfrom
Issue-233-Cfg-Property-And-Document-Parent-Init-Call

Conversation

@eldipa
Copy link
Copy Markdown
Collaborator

@eldipa eldipa commented May 14, 2022

Closes #233

eldipa added 24 commits May 12, 2022 03:56
init_byexample is in charge to create a definitive (almost) cfg object.
The refactor moves all the changes to the object as sooner as possible
to make it clear from where cfg could be considered a constant object.
…licitly (API break)

ExampleFinder, ExampleParser, ExampleRunner, ZoneDelimiter and Concern
inherit from Extension. This super class is initialized (__init__) with
a single keyword-only parameter: cfg (the Config).

Any other keyword  parameter is accepted but discarded (to reduce the
amount of changes to the API).

Subclasses must call their parent's __init__ passing all the keyword
arguments (**kargs). This will ensure that the cfg passed will reach to
Extension.__init__.

However this is not enforced (yet).

This commit breaks the API because the constructors now require
a mandatory cfg.

So we used to have:

    parser = ExampleParser(0, 'utf8', Options())

And now we require

    from byexample.cfg import Config
    parser = ExampleParser(cfg=Config(
                verbosity=0,
                encoding='utf8',
                options=Options()
             ))

Note: it is expected that this change will have little impact on
practice as the extension classes are instantiated by byexample
and this commit takes care of passing the correct object as cfg.
…reak)

These are legacy and not used in all the cases. Now, the generic cfg
(Config) can carry any amount of attributes and if a class/subclass
needs one, it can query it.

This could break some extension.

In case if need, the attribute can be get from __init__ as

class MyParser(ExampleParser):
    def __init__(self, verbosity, encoding, **kargs):
        super().__init__(verbosity=verbosity, encoding=encoding, **kargs)

        self.verbosity = verbosity
        self.encoding = encoding

Or through the cfg property:

class MyParser(ExampleParser):
    def __init__(self, **kargs):
        super().__init__(**kargs)

        self.verbosity = self.cfg.verbosity
        self.encoding = self.cfg.encoding

Most of the use cases for overriding __init__ is to "capture" one or
more settings. This is not needed anymore with the cfg property.
In the same way we pass the namespace ns outside of cfg to the
extensions, the sharer is passed in the same way, outside of cfg.

This makes more consistency between ns and sharer, and between the main
thread and thw worker threads.
…API break)

Any subclass of ExampleFinder, ExampleParser, ExampleRunner,
ZoneDelimiter and Concern must call their parent __init__ method passing
all the arguments received.

If the extension requires ns and or sharer, they can access them via
keyword only arguments. They will not be present in the cfg because they
are quite special.

This breaks the API. Developers must invoke the __init__ of the parent
class.
…didn't call super __init__ (API break)

PexpectMixin was never designed as an independent mixin and small details
of the ExampleRunner are leaked.

Making PexpectMixin independent of ExampleRunner would require passing
more arguments to PexpectMixin.__init__ which will make the API break
more noticeable. (and the __init__ arguments will probably look more
awkward)

In practice, requiring PexpectMixin be inherited by a class that also
inherits from ExampleRunner is not unusual.

Requiring that PexpectMixin.__init__ be called after
ExampleRunner.__init__ is more likely to break but also it is easy to
fix.
…break)

Because some extension class, like ExampleRunner, now don't define its
own __init__, they don't set any attribute so subclasses cannot assume
that such attribute is defined.

Instead, they can relay on self.cfg

For example, a subclass of ExampleRunner cannot assume self.encoding to
exists but it can assume that self.cfg.encoding will.
…ll not set after extension instantiation (API break)

Before this commit, byexample ignored any class that didn't have a
target/language even if the class inherited from an extension class
like ExampleParser. Just a warning was issued.

Now, byexample will load the class anyways but it will require to have a
target/language defined. If no such attribute exists, it will raise an
exception.

The extension class can not define it but its __init__ must ensure
it is defined. This allows the extension to dynamically change its
target/language based on some configuration/options or even to disable
itself setting it to None.

ProgressBarReporter and SimpleReporter are examples of this.

This commits breaks the API (new classes are loaded) but it is unlikely
that it will break real code.
…n cfg retrieve

If a subclass' __init__ uses the cfg property without having called
Extension.__init__, raise an AttributeError describing why the cfg is
not set yet and what to do.

This should hint the developer to catch an error early.
@eldipa eldipa added this to the 11.0.0 milestone May 14, 2022
@eldipa eldipa force-pushed the Issue-233-Cfg-Property-And-Document-Parent-Init-Call branch from 1013bb3 to cf5576d Compare May 14, 2022 22:17
@eldipa eldipa merged commit 0957935 into master May 14, 2022
@eldipa eldipa deleted the Issue-233-Cfg-Property-And-Document-Parent-Init-Call branch May 14, 2022 22:29
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

Successfully merging this pull request may close these issues.

Introduce cfg and document that the call to parent's __init__ is mandatory for ExampleParser, ExampleRunner, ExampleFinder, ZoneDelimiter and Concern

1 participant