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

Attempting to polyfill the decorate() function #1702

Merged
merged 2 commits into from Mar 6, 2017

Conversation

Projects
None yet
2 participants
@krischer
Member

krischer commented Mar 6, 2017

See #1644.

#1671 did fix the test failures but the decorator in this case did not actually do anything as the lock was recreated for each function call and thus there was no lock.

This is an attempt to polyfill the decorate() function to older decorator modules as that functionality is crucial for a lot of things that don't really work with normal decorators like our documentation.

I honestly have no idea how to actually test this - one would need to get access to the local variables of the decorated function but __closure__ appears to be empty. A found a solution online but that seems a bit too much hassle to actually do it: http://stackoverflow.com/questions/9186395/python-is-there-a-way-to-get-a-local-function-variable-from-within-a-decorator

I tested this with print statements and it appears to work as intended.

@krischer krischer changed the title from Attempting to polyfill the decorate() function. to Attempting to polyfill the decorate() function Mar 6, 2017

@megies

This comment has been minimized.

Show comment
Hide comment
@megies

megies Mar 6, 2017

Member

#1671 did fix the test failures but the decorator in this case did not actually do anything as the lock was recreated for each function call and thus there was no lock.

Sorry.. was relying on test results there as I wasn't familiar with the details.

Member

megies commented Mar 6, 2017

#1671 did fix the test failures but the decorator in this case did not actually do anything as the lock was recreated for each function call and thus there was no lock.

Sorry.. was relying on test results there as I wasn't familiar with the details.

@megies megies added this to the 1.1.0 milestone Mar 6, 2017

@megies megies added the .core.event label Mar 6, 2017

@megies megies referenced this pull request Mar 6, 2017

Merged

Object aware ResourceIdentifier #1644

6 of 6 tasks complete
@krischer

This comment has been minimized.

Show comment
Hide comment
@krischer

krischer Mar 6, 2017

Member

No problem - and it also is a bit tricky and not immediately obvious + not tested...

You are also not the only one ;-) 18dbddb#diff-20a7c5c57de612451c7de805bc4addecR299

Member

krischer commented Mar 6, 2017

No problem - and it also is a bit tricky and not immediately obvious + not tested...

You are also not the only one ;-) 18dbddb#diff-20a7c5c57de612451c7de805bc4addecR299

@krischer

This comment has been minimized.

Show comment
Hide comment
@krischer

krischer Mar 6, 2017

Member

Yay :) Seems to work! Even though the decorator is not directly tested it is executed quite a bit in the whole test suite so I conclude that it works as expected.

Member

krischer commented Mar 6, 2017

Yay :) Seems to work! Even though the decorator is not directly tested it is executed quite a bit in the whole test suite so I conclude that it works as expected.

@krischer krischer merged commit 900840a into obspy:master Mar 6, 2017

5 of 7 checks passed

codecov/changes 55 files have unexpected coverage changes not visible in diff.
Details
codecov/patch 56.25% of diff hit (target 90%)
Details
ci/circleci Your tests passed on CircleCI!
Details
codecov/project 86.96% (+0.92%) compared to afd294b
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
docker-testbot Docker tests succeeded
Details

@krischer krischer deleted the krischer:rlock-decorator-fix branch Mar 6, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment