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

Allow setting and resetting the global adaptation manager. #145

Merged
merged 8 commits into from
Feb 25, 2014

Conversation

pberkes
Copy link
Contributor

@pberkes pberkes commented Feb 24, 2014

The global adaptation manager is used by traits for automatic
adaptation. Setting and resetting it is useful at application
start-up, but especially during testing to reset the global
state.

The global adaptation manager is used by traits for automatic
adaptation. Setting and resetting it is useful at application
start-up, but especially during testing to reset the global
state.
@enbuilder
Copy link

Can one of the admins verify this patch?

#: PROVIDED FOR BACKWARD COMPATIBILITY ONLY, IT SHOULD NEVER BE USED DIRECTLY.
#: If you must use a global adaptation manager, use the functions
#: `get_global_adaptation_manager`, `reset_global_adaptation_manager`,
#: `set_global_adaptation_manager`.
adaptation_manager = AdaptationManager()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we wrap access to this attribute and throw a deprecation warning, or is it an overkill?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just complicated to do: you need to wrap the module in an object... I'd say it's overkill.
adaptation_manager is not exposed through the api modules, so you need to really look for it in order to get it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair enough

@pberkes
Copy link
Contributor Author

pberkes commented Feb 24, 2014

I broke something, looking into it.

@pberkes
Copy link
Contributor Author

pberkes commented Feb 24, 2014

Fixed, and updates CHANGES and docs.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling a196c9b on fix/global-adaptation-manager into ec5fe8c on master.

function
:func:`~traits.adaptation.adaptation_manager.get_global_adaptation_manager`.
The traits automatic adaptation features also use the global manager.
Having a global adaptation manager can get you into troubles, for the usual
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

troubles -> trouble

@itziakos
Copy link
Member

👍

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling ef7ec8f on fix/global-adaptation-manager into ec5fe8c on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling ef7ec8f on fix/global-adaptation-manager into ec5fe8c on master.

adaptation_manager.register_factory(
factory = ex.UKStandardToEUStandard,
from_protocol = ex.UKStandard,
to_protocol = ex.EUStandard
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be very fussy to request an extra comma here? If so, I won't.

@mdickinson
Copy link
Member

LGTM; I've left some comments, but I'm happy for this to be merged as it is.

@pberkes
Copy link
Contributor Author

pberkes commented Feb 25, 2014

@mdickinson I fixed the details, but if it's of with you I'd like to leave the rest unchanged.

@mdickinson
Copy link
Member

Sure. I'll wait for the CI and then merge.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 73fdb51 on fix/global-adaptation-manager into ec5fe8c on master.

mdickinson added a commit that referenced this pull request Feb 25, 2014
Allow setting and resetting the global adaptation manager.
@mdickinson mdickinson merged commit eb6a4da into master Feb 25, 2014
@mdickinson mdickinson deleted the fix/global-adaptation-manager branch February 25, 2014 09:06
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.

5 participants