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

restore hardcoded config_dir #905

Merged
merged 1 commit into from
Dec 31, 2015

Conversation

minrk
Copy link
Member

@minrk minrk commented Dec 29, 2015

reverts #882
closes #893

@Carreau
Copy link
Member

Carreau commented Dec 30, 2015

ping @damianavila @ellisonbg. I had something similar with more complex PendingDeprecation warning for the kwarg (that it will have no effects)

@Carreau
Copy link
Member

Carreau commented Dec 30, 2015

Also
+1, let's merge this and move forward on 4.1

@minrk minrk added this to the 4.1 milestone Dec 30, 2015
@damianavila
Copy link
Member

That's OK, I still believe we are hiding a bug but since the things are going to change for 4.2, we can live with it... in fact, it is just a matter to load the proper config_dir at the __init__ of the custom Config Manager (or use Min workaround for more safety)

@minrk
Copy link
Member Author

minrk commented Dec 30, 2015

It is hiding a bug in the original intention, but as @ellisonbg mentioned, the bug is that a feature doesn't work that perhaps never should work, and the fact that it hasn't ever worked lets us move forward as if we've never had that feature (if that makes any sense at all).

@ellisonbg
Copy link
Contributor

That is a good summary @minrk

However, the more important thing is figuring out what @damianavila et al want to do with their custom ConfigManager. They could, for example, use that functionality to enable the ConfigManger from my 4.2 targeted PR.

But thinking further, because my custom ConfigManager doesn't have a config_dir attribute, hardcoding config_dir in NotebookApp wouldn't prevent them from using it.

@damianavila
Copy link
Member

  1. We can live with this with our custom ConfigManager for now...
  2. My idea is to start migrating to something like Brian ConfigManager proposal, ASAP (as time permits).
  3. We would like to be fully compatible with the consensus/ideas we achieved for 4.2

@ellisonbg
Copy link
Contributor

If you move to using the custom ConfigManager that I have in #879 then it
matters less what we do with the config_dir stuff in 4.1.

Looking forward - the ConfigManager I have in my PR doesn't even have a
config_dir attribute. What is the best way to ship 4.1 that reflects that
fact?

On Wed, Dec 30, 2015 at 11:33 AM, Damián Avila notifications@github.com
wrote:

  1. We can live with this with our custom ConfigManager for now...
  2. My idea is to start migrating to something like Brian ConfigManager
    proposal, ASAP (as time permits).
  3. We would like to be fully compatible with the consensus/ideas we
    achieved for 4.2


Reply to this email directly or view it on GitHub
#905 (comment).

Brian E. Granger
Associate Professor of Physics and Data Science
Cal Poly State University, San Luis Obispo
@ellisonbg on Twitter and GitHub
bgranger@calpoly.edu and ellisonbg@gmail.com

@damianavila
Copy link
Member

Looking forward - the ConfigManager I have in my PR doesn't even have a
config_dir attribute. What is the best way to ship 4.1 that reflects that
fact?

I think we just need to keep the things as is (after merging this one)... the "fact" is 4.2 so why we should have to reflect that in 4.1, I think we are just trying to make bets on a future facts 😉
Since we are so close to 4.1, let's just merge this one as is and we fix all of this for 4.2, how that's sound?

@minrk
Copy link
Member Author

minrk commented Dec 31, 2015

Makes sense to me. That way 4.1 doesn't have any changes from 4.0, and there are only two states to consider: before 4.2, after 4.2.

minrk added a commit that referenced this pull request Dec 31, 2015
@minrk minrk merged commit 1257a3a into jupyter:master Dec 31, 2015
@minrk minrk deleted the revert-configurable-config-dir branch December 31, 2015 10:42
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove configurability of config_dir in nbconfig
4 participants