Skip to content

Remove AutoPropertyObject dependency on __init__#7833

Merged
feerrenrut merged 1 commit into
masterfrom
fixUninitialisedBaseClass
Dec 21, 2017
Merged

Remove AutoPropertyObject dependency on __init__#7833
feerrenrut merged 1 commit into
masterfrom
fixUninitialisedBaseClass

Conversation

@feerrenrut
Copy link
Copy Markdown
Contributor

@feerrenrut feerrenrut commented Dec 6, 2017

Link to issue number:

No issue number.

Summary of the issue:

A class that inherits from AutoPropertyObject and does not (forgets to) call super in the __init__ function, would result in a failure for CachedGetter properties.
An example of this is BrailleBuffer

To reproduce the error, in the NVDA python console:

import braille
braille.handler.buffer.brailleToRawPos

gives:

braille.handler.buffer.brailleToRawPos
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "baseObject.pyc", line 34, in __get__
  File "baseObject.pyc", line 108, in _getPropertyViaCache
AttributeError: 'BrailleBuffer' object has no attribute '_propertyCache'

Description of how this pull request fixes the issue:

Create the _cachedProperties dictionary in the __new__function, this is always called automatically.
A missing super call on AutoPropertyObject children no longer causes failure for CachedGetter properties.

While here I noticed that there is the possibility that the generator expression for transforming the dictionary keys (such as _get_myVal, _set_myVal, and _del_myVal) would result in duplicates ("myVal" would be output 3 times). This results in unnecessary iterations of the loop that follows. Converting this to a set comprehension avoids that.

Testing performed:

Ran NVDA and ran the following in the NVDA python console without getting an error:

import braille
braille.handler.buffer.brailleToRawPos

Known issues with pull request:

None

Change log entry:

None

Missing super call on AutoPropertyObject children no longer causes
failure for CachedGetter properties. Previously a class that inherits
from AutoPropertyObject and forgot to call super in the __init__
function, would result in a failure for CachedGetter properties.
feerrenrut added a commit that referenced this pull request Dec 6, 2017
…ewer

This implementation of brailleViewerTool relies on #7833 being fixed
feerrenrut added a commit that referenced this pull request Dec 6, 2017
Merge remote-tracking branch 'origin/pr/7833' into next
@derekriemer
Copy link
Copy Markdown
Collaborator

Might this hide any errors if the addon author orgets to call super in init

@feerrenrut
Copy link
Copy Markdown
Contributor Author

If they are inheriting directly from AutoPropertyObject then there is now no risk to forgetting to call super. However, for other classes this danger essentially has not changed. Before this change, it was possible that everything would work correctly for most of the time. Only when get was called on specific objects (those that are cached) would the error show up. How this problem manifests really depends on the object and how it's used.

@feerrenrut feerrenrut merged commit b28fe86 into master Dec 21, 2017
@nvaccessAuto nvaccessAuto added this to the 2018.1 milestone Dec 21, 2017
@feerrenrut feerrenrut deleted the fixUninitialisedBaseClass branch January 17, 2020 09:00
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.

4 participants