ENH: Make set_option into a contextmanager that undos itself on __exit__

[jtratner]

Fixes #5618

cc @y-p and @jseabold

I went ahead and simplifed all the prefixing stuff to just change a single prefix_key function instead and handled the dynamic docstring by setting the __doc__ attribute of functions/classes. set_option is now callable and a contextmanager (whee!) so no need to add anything else to the global namespace (if you don't call __exit__ it doesn't undo itself)

Had to add a metaclass to this because it's the only way to set docstrings on the class level. Doesn't really add that much noise so I think it's fine.

[ghost]

• The prefix stuff is unrelated, please break it out into a seperate commit.
• Need to update docs about set_option as context manager, and the doctsring + release notes,

I don't have a problem with the metaclass here, it seems like the only way and clean enough.
With docs, +1 to merge that bit.

• What's the reason for the changes around config_prefix? I was unaware of any problem there, is
  there additional functionality you haven't mentioned?
• Why did you replace a self-contained context manager which wraps existing functions without modification
  by an implementation that introduces a global mutable variable (the prefix_key method) and requires
  modification to all the existing function for cooperation?
• You've added an undocumented prefix keyword in multiple places, including the public API.
  What's it for? is it for users or internal use?

[ghost]

Note that the dynamic docstring code I implemented solves a problem that we never had.
The docstring doesn't change after config_init runs, It just needs to be generated once
at the correct moment.

In [8]: #import option_docstring from pd.core.config_init
   ...: option_docstring = "foo"
   ...: 
   ...: class set_option(object):
   ...:     __doc__ = option_docstring
   ...:     def boo(self): 
   ...:         pass
   ...: a=Foo()
   ...: 

In [9]: a.__doc__
Out[9]: 'foo'

Look ma, no metaclasses.

[jtratner]

@y-p okay.

In working on this, I realized that config_prefix was actually broken, you can pass: set_option(pat, val, pat, val, pat, val) and only the first pat was being prefixed (everything else was just passed through). It was already modifying global variables, given that it was swapping out set_option, get_option, register_option, etc (such that you couldn't use from imports with the module), so it seemed simpler to only swap out the prefix_key method. I'll come up with a way to not need to use a separate prefix keyword but still be able to keep track of what to undo.

I guess this also doesn't work with nested config_prefix calls, so I should fix that too.

[ghost]

That makes sense, the support for additional args was added only later, which created the potential
problem. All the existing code (tests only?) should obey the original limitations, though, I don't think
it manifests anywhere..

Anyway, the set_option context manager bit is good, murge iiit!

[jtratner]

@y-p do you want me to split the changes to config_prefix apart from making set_option into a contextmanager?

[ghost]

If you can, that'd be better. it's a seperate issue.

[jreback]

0.14?

[jtratner]

yes.

On Thu, Dec 5, 2013 at 6:43 PM, jreback [email protected] wrote:

0.14?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/5625#issuecomment-29950645
.

[ghost]

@jtratner, apologies if my notes were too fussy. Remember...

...When someone rewrites my code.

[jreback]

new logo :)

[cpcloud]

That's great!

[jtratner]

@y-p - no no it's just because I've been incredibly busy (interviewing and
now preparing to move across the country for a new job). Definitely legit
to split into two parts and I want to get to it. I'm planning to get in
some pandas work this weekend / after the holidays so I'll get to it then.

[jtratner]

also that image is great!

[ghost]

thecodinglove.com borders on medically required at times.

@jtratner, I'm working to close a bunch of little stuff to earn my keep around here.
If you're tied up, mind if I rip out just the context manager bit into a PR to just
get that in? Weekend is great too, if it suits your plans.

[jtratner]

go ahead and do it - fine with me! I really want to finish up the
RangeIndex and the index refactoring anyway, so it'll be nice not to have
this on my plate :)

[jreback]

OT: anyone thing ought to do a rc2? (or put one up for say 5 days, with no more changes) (I wont! :)

seems windows builds are keeping up (@y-p your doing?)

[jtratner]

I'd be +1 on rc2. I would also strongly advocate for freeze on features for
this rc2 so we can take off the pressure on all some of the pending PRs
(maybe we should actually branch things off like I put up in that issue?)

[ghost]

I was thinking the exact same thing. RC2 or just push back final until no reports come in for a fix num of days.

@jtratner, good I'll take it.
@jreback, not me, I'll just (sporadically) open an issue if something breaks. I'm still working
on a better solution there.

also, I've been nagging about a feature freeze since rc1 anyway, I thought we already had one in effect? :)

[jreback]

@y-p I know ...been trying to just do bug fixes...have to draw the line though

[jtratner]

@y-p maybe one of us should email pandas-dev to announce an official feature freeze? (and then possibly create a maintenance branch for 0.13)

[jtratner]

I'm happy to write up the email if there's agreement.

[jreback]

@jtratner I sent an e-mail about rc2 to pandas-dev...you didn't get it?

[jtratner]

no, I didn't see anything...

[jreback]

@jtratner weird......gmail claims it was sent.....

[jtratner]

just got it now

[ghost]

I would argue that if we can keep releases timely, we can avoid working on multiple branches.

In the past there were 0.x.1 bugfix releases but that hasn't happened in a while and things haven't collapsed,
so arguably we can live without them, we just need to fight release date limbo and do a proper rc period
to flush out lethal sleeper bugs. You don't agree?

[jreback]

I agree.....

This has been a 'bigger' release simply because more moving parts.

So far so good IMHO, no major reports (of course prob not THAT widespread use for a rc)...but did get some reports.

as I said on the mail-list.....I think rc2 would be worthwhile for say a week or so.

[jtratner]

well, let's see how we do in the next few days. I think it's hard to tell
someone who's posted a PR that they need to wait. Just makes it easier to
do a feature freeze if we let master move forward.

[jtratner]

oh, if it's just another week then no need for separate branch. I was
thinking we were going to take a month or something like that.

[jreback]

I think could release now, but let's giev a rc2 just for anything else to come up

[ghost]

PRs on hold for two-three weeks is not that bad, some of them take a week or two of iteration anyway.
If releases dates are predictable everything else should fall into place, that's the theory and that's why
it's important.

[ghost]

closing in favor of #5752