Issue 233 cfg property and document parent init call#234
Merged
Conversation
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.
1013bb3 to
cf5576d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #233