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

Implement hasattr_warn(); run to remove hasattr(prop) calls #362

Closed
trevorbaca opened this issue Mar 22, 2014 · 4 comments
Closed

Implement hasattr_warn(); run to remove hasattr(prop) calls #362

trevorbaca opened this issue Mar 22, 2014 · 4 comments
Assignees
Labels

Comments

@trevorbaca
Copy link
Member

trevorbaca commented Mar 22, 2014

We've experienced too many relatively significant bugs related to calling hasattr() on properties.

We need a way to audit the entire codebase and remove all cases of calling hasattr() on properties.

How to do this?

We don't need anything superelegant. Just something that will let us run over the codebase and go manually fix everything found.

One solution would be to implement a custom hasattr_warn() function. The function would raise an exception in the case that it's input argument is a property. Probably define directly in the abjad/__init__.py module so that we can then add the function to the __builtins__ namespace with something like __builtins__['hasattr_warn'] = hasattr_warn. Then we could globally search and replace all hasattr() with hasattr_warn() and then run and fix the tests iteratively until everything runs and no more exceptions are raised. Then globally search and replace to change all hasattr_warn() calls back to vanilla hasattr().

trevorbaca referenced this issue Mar 22, 2014
Consider the following code:

    measure = Measure((4, 4), "c'4")
    staff = Staff([measure])
    score = Score([staff])
    score_block = lilypondfiletools.Block(name='score')
    score_block.items.append(score)

The above measure is underfull. Formatting it should result in an
UnderfullContainerError. And in fact if we format the measure, the staff or the
score we'll raise such an error.

However, until this commit, formatting the score block would raise no error and
would produce the following LilyPond syntax:

    \score {
    }

Why is this? It's all due to Python's hasattr implementation. When formatting
itself, Block calls hasattr on each item in its items array, testing if they
have anything like `_get_format_pieces` or `_format_pieces` that it could use
to construct some LilyPond syntax. Python's hasattr not only executes each
attribute it tests for but also traps any exceptions raised in the process.

And not only does it trap exceptions, but it also returns False if it *does*
encounter an exception. All together this means that we never find out about
our malformed measure, and our score block assumes that the score it contains
has no LilyPond format, so nothing gets formatted either.

This has been fixed by testing for the presence of `_format_pieces` in the
dir() of each item in the block's item array, and now exceptions are raised
again.
trevorbaca referenced this issue Apr 8, 2015
NOTE: Be careful with hasattr(). This function consumes exceptions silently,
returning false instead.
@trevorbaca
Copy link
Member Author

Still valid in June 2017.

@trevorbaca trevorbaca changed the title Implement hasattr_warn() and run over codebase to remove hasattr(prop) calls Implement hasattr_warn(); run to remove hasattr(prop) calls Jul 3, 2017
@trevorbaca
Copy link
Member Author

A hasattr_warn() replacement now exists in the Abjad __init__.py file. Just uncomment and run the tests to see what fails.

Current doctests:

12492 passed, 2654 failed out of 15146 tests in 821 modules.

Current pytests:

813 failed, 9349 passed, 523 skipped in 226.93 seconds

@trevorbaca trevorbaca self-assigned this Jul 4, 2017
@trevorbaca
Copy link
Member Author

Current doctests:

14890 passed, 256 failed out of 15146 tests in 821 modules.

Current pytests:

81 failed, 10081 passed, 523 skipped in 198.14 seconds

@trevorbaca
Copy link
Member Author

Done in 2.21.

The following remains in the Abjad __init__.py for future housecleaning:

def hasattr_warn(argument, name, original_hasattr=hasattr):
    if original_hasattr(argument.__class__, name):
        value = getattr(argument.__class__, name)
        if isinstance(value, property):
            message = 'WARNING: {}.{} is a property!'
            message = message.format(argument.__class__.__name__, name)
            raise Exception(message)
    return original_hasattr(argument, name)
__builtins__['hasattr'] = hasattr_warn

trevorbaca added a commit that referenced this issue Jul 31, 2017
trevorbaca added a commit that referenced this issue Jul 31, 2017
trevorbaca added a commit that referenced this issue Jul 31, 2017
(#362) Removed several hasattr(argument, property) calls.
(#362) Removed more hasattr(argument, property) calls.
(#362) Removed all hasattr(argument, property) calls.
This closes #362.
trevorbaca added a commit that referenced this issue Jul 31, 2017
(#362) Removed several hasattr(argument, property) calls.
(#362) Removed more hasattr(argument, property) calls.
(#362) Removed all hasattr(argument, property) calls.
This closes #362.
trevorbaca added a commit that referenced this issue Jul 31, 2017
Added (commented-out) hasattr_warn() to Abjad __init__.py.
trevorbaca added a commit that referenced this issue Jul 31, 2017
Added (commented-out) hasattr_warn() to Abjad __init__.py.
trevorbaca added a commit that referenced this issue Jul 31, 2017
Added (commented-out) hasattr_warn() to Abjad __init__.py.
trevorbaca added a commit that referenced this issue Jul 31, 2017
Added (commented-out) hasattr_warn() to Abjad __init__.py.
trevorbaca added a commit that referenced this issue Jul 31, 2017
Added (commented-out) hasattr_warn() to Abjad __init__.py.
trevorbaca added a commit that referenced this issue Jul 31, 2017
Added (commented-out) hasattr_warn() to Abjad __init__.py.
trevorbaca added a commit that referenced this issue Jul 31, 2017
Added (commented-out) hasattr_warn() to Abjad __init__.py.
trevorbaca added a commit that referenced this issue Jul 31, 2017
Added (commented-out) hasattr_warn() to Abjad __init__.py.
trevorbaca added a commit that referenced this issue Jul 31, 2017
Added (commented-out) hasattr_warn() to Abjad __init__.py.
trevorbaca added a commit that referenced this issue Jul 31, 2017
Added (commented-out) hasattr_warn() to Abjad __init__.py.
trevorbaca added a commit that referenced this issue Jul 31, 2017
Added (commented-out) hasattr_warn() to Abjad __init__.py.
trevorbaca added a commit that referenced this issue Jul 31, 2017
Added (commented-out) hasattr_warn() to Abjad __init__.py.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants