Skip to content

Simplify context manager for enabling units - #1662

Merged
mdboom merged 2 commits into
astropy:masterfrom
mdboom:unit/simpler-context-manager
Oct 24, 2013
Merged

mdboom merged 2 commits into
astropy:masterfrom
mdboom:unit/simpler-context-manager

Conversation

@mdboom

@mdboom mdboom commented Oct 23, 2013

Copy link
Copy Markdown
Contributor

It has always bugged me that we had separate functions for enabling
units permanently and temporarily. This merges set_enabled_units
and set_enabled_units_context into a single function by using a
custom context manager rather than contextlib.contextmanager.

It would be great to have this small change in 0.3, as it affects API.

@astrofrog

Copy link
Copy Markdown
Member

Sounds good to me - looks like there's a minor whitespace warning in the docs?

@mhvk

mhvk commented Oct 23, 2013

Copy link
Copy Markdown
Contributor

@mdboom - while doing #1593, I had a pretty good look over the code; as said there, it certainly looks fine. I now went over the other files and those look good too.

@embray

embray commented Oct 24, 2013

Copy link
Copy Markdown
Member

Yeah, assuming no one else has any problem this looks much better.

@astrofrog

Copy link
Copy Markdown
Member

@mdboom - feel free to merge!

mdboom added a commit that referenced this pull request Oct 24, 2013
Simplify context manager for enabling units
@mdboom
mdboom merged commit a6149c0 into astropy:master Oct 24, 2013
@mdboom
mdboom deleted the unit/simpler-context-manager branch May 21, 2014 23:58
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