Remove %install_default_config and %install_profiles #1174

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
@tomspur
Contributor

tomspur commented Dec 18, 2011

as they are now controllen with "ipython profile" commands.

Remove %install_default_config and %install_profiles
as they are now controllen with "ipython profile" commands.

closes #1174 and Redhad bug #751146

Signed-off-by: Thomas Spura <thomas.spura@gmail.com>
@fperez

This comment has been minimized.

Show comment
Hide comment
@fperez

fperez Dec 18, 2011

Member

@takluyver, @minrk: if you guys can have a look at this, go for it. I have to finish a talk for work today and will barely make all the last-minute work for the actual release.

Member

fperez commented Dec 18, 2011

@takluyver, @minrk: if you guys can have a look at this, go for it. I have to finish a talk for work today and will barely make all the last-minute work for the actual release.

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Dec 18, 2011

Member

Sure, though I think these were added at your request, so this would remove the (obvious, easy) ability to generate the default config files from IPython.

Member

minrk commented Dec 18, 2011

Sure, though I think these were added at your request, so this would remove the (obvious, easy) ability to generate the default config files from IPython.

@fperez

This comment has been minimized.

Show comment
Hide comment
@fperez

fperez Dec 18, 2011

Member

Mmhh... I don't recall, but still, it's a good point. The question is: when we added this, did we have any mechanism at all other than manual copying of files and directories? I think we didn't, so a magic seemed like a good solution. Furthermore, I don't think our old system recognized profiles that were kept in the ipython directory, only those that had been made available to the user.

Now that users can specify with --profile a profile that they haven't installed locally, it seems to me that there is much less need to offer to install profiles in their personal area (if anything, there's less incentive, as by not installing their personal copies they benefit from updates we make to the shipped ones).

Member

fperez commented Dec 18, 2011

Mmhh... I don't recall, but still, it's a good point. The question is: when we added this, did we have any mechanism at all other than manual copying of files and directories? I think we didn't, so a magic seemed like a good solution. Furthermore, I don't think our old system recognized profiles that were kept in the ipython directory, only those that had been made available to the user.

Now that users can specify with --profile a profile that they haven't installed locally, it seems to me that there is much less need to offer to install profiles in their personal area (if anything, there's less incentive, as by not installing their personal copies they benefit from updates we make to the shipped ones).

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Dec 18, 2011

Member

An alternative, if we want to fix this behavior, would be:

loc = self.shell.profile_dir.location
cmd = ["ipython", "profile", "create", "--ProfileDir.location=%r" % loc]
if '-o' in s:
    cmd += '--reset'
subprocess.call(cmd)

And for %install_profiles:

if '-o' in s:
    overwrite = True
else:
    overwrite = False
from IPython.config import profile
profile_dir = os.path.dirname(profile.__file__)
ipython_dir = self.ipython_dir
print "Installing profiles to: %s [overwrite=%s]"%(ipython_dir,overwrite)
for name in os.listdir(profile_dir):
    src = os.path.join(profile_dir, name)
    files = glob.glob(os.path.join(src, "*.py"))
    if files:
        print "    %s" % name
        pd = ProfileDir.create_profile_dir_by_name(ipython_dir, name)
        for fname in [ os.path.basename(f) for f in files ]:
            pd.copy_config_file(fname, path=src, overwrite=overwrite)

If we do remove these magics, we should probably skim the docs for mentions, so we aren't pointing to them anymore.

Member

minrk commented Dec 18, 2011

An alternative, if we want to fix this behavior, would be:

loc = self.shell.profile_dir.location
cmd = ["ipython", "profile", "create", "--ProfileDir.location=%r" % loc]
if '-o' in s:
    cmd += '--reset'
subprocess.call(cmd)

And for %install_profiles:

if '-o' in s:
    overwrite = True
else:
    overwrite = False
from IPython.config import profile
profile_dir = os.path.dirname(profile.__file__)
ipython_dir = self.ipython_dir
print "Installing profiles to: %s [overwrite=%s]"%(ipython_dir,overwrite)
for name in os.listdir(profile_dir):
    src = os.path.join(profile_dir, name)
    files = glob.glob(os.path.join(src, "*.py"))
    if files:
        print "    %s" % name
        pd = ProfileDir.create_profile_dir_by_name(ipython_dir, name)
        for fname in [ os.path.basename(f) for f in files ]:
            pd.copy_config_file(fname, path=src, overwrite=overwrite)

If we do remove these magics, we should probably skim the docs for mentions, so we aren't pointing to them anymore.

@fperez

This comment has been minimized.

Show comment
Hide comment
@fperez

fperez Dec 18, 2011

Member

On Sun, Dec 18, 2011 at 12:26 PM, Min RK
reply@reply.github.com
wrote:

If we do remove these magics, we should probably skim the docs for mentions, so we aren't pointing to them anymore.

My only concern with removing them is braking existing habits.
Perhaps it would be best to replace them with a print statement that
explains the new profile behavior (automatic access to builtin
profiles, etc) and how to create one.

Thoughts?

Member

fperez commented Dec 18, 2011

On Sun, Dec 18, 2011 at 12:26 PM, Min RK
reply@reply.github.com
wrote:

If we do remove these magics, we should probably skim the docs for mentions, so we aren't pointing to them anymore.

My only concern with removing them is braking existing habits.
Perhaps it would be best to replace them with a print statement that
explains the new profile behavior (automatic access to builtin
profiles, etc) and how to create one.

Thoughts?

@tomspur

This comment has been minimized.

Show comment
Hide comment
@tomspur

tomspur Dec 18, 2011

Contributor

Pointing to the new profile behavior would be fine too.

I'd delete them and point it out in the release notes, so people can find it through your favorite search engine.
But pointing to the new profile behavior for a few releases and then it can be cleaned up is fine too...

Contributor

tomspur commented Dec 18, 2011

Pointing to the new profile behavior would be fine too.

I'd delete them and point it out in the release notes, so people can find it through your favorite search engine.
But pointing to the new profile behavior for a few releases and then it can be cleaned up is fine too...

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Dec 18, 2011

Member

I like the print statement. ipython profile create is definitely better than the %install_default_config, but we don't really have an analog to %install_profiles, which stages all the bundled profiles. That said, I'm not sure how many people actually care about that one, as you can just use profiles, and the first time you ask for them, the default is staged.

Member

minrk commented Dec 18, 2011

I like the print statement. ipython profile create is definitely better than the %install_default_config, but we don't really have an analog to %install_profiles, which stages all the bundled profiles. That said, I'm not sure how many people actually care about that one, as you can just use profiles, and the first time you ask for them, the default is staged.

@fperez

This comment has been minimized.

Show comment
Hide comment
@fperez

fperez Dec 18, 2011

Member

I'd be OK with %install_profiles being replaced also with a print statement that explains how one can use any bundled profile. I'm not sure there's much value/need for the 'dump all profiles automatically to my ipython dir'.

Member

fperez commented Dec 18, 2011

I'd be OK with %install_profiles being replaced also with a print statement that explains how one can use any bundled profile. I'm not sure there's much value/need for the 'dump all profiles automatically to my ipython dir'.

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Dec 18, 2011

Member

The only advantage I see of %install_profiles is that it affects the output of ipython profile list to see everything available, but I'm not sure anyone uses that, not least of all because I just saw that the output was actually never fixed since we changed the way we specify args prior to 0.11 release.

It is currently the only easy way we have of seeing what the list of bundled profiles is. But since it would appear that people aren't really using it that way, I don't think anyone would be sad to see it go.

Member

minrk commented Dec 18, 2011

The only advantage I see of %install_profiles is that it affects the output of ipython profile list to see everything available, but I'm not sure anyone uses that, not least of all because I just saw that the output was actually never fixed since we changed the way we specify args prior to 0.11 release.

It is currently the only easy way we have of seeing what the list of bundled profiles is. But since it would appear that people aren't really using it that way, I don't think anyone would be sad to see it go.

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Dec 18, 2011

Member

I should also clarify that viewing the available bundled profiles is the only benefit to %install_profiles. There is exactly no difference between doing ipython --profile=sympy before/after %install_profiles, because bundled config is staged on first request of the profile anyway.

Member

minrk commented Dec 18, 2011

I should also clarify that viewing the available bundled profiles is the only benefit to %install_profiles. There is exactly no difference between doing ipython --profile=sympy before/after %install_profiles, because bundled config is staged on first request of the profile anyway.

@fperez

This comment has been minimized.

Show comment
Hide comment
@fperez

fperez Dec 18, 2011

Member

Shouldn't we then instead just fix profile list to show the existing ones from IPython? I imagine something like

ipython profile list

Available profiles in IPython:
- foo
- bar

Available profiles in /path/to/your/config/ipython:
-baz
-zap

Local profiles in the current directory (/path/to/cwd):
- x
- y

Note: you can use any of these by using `ipython profile --profile <name>`; 
if it is one of those within ipython it will be automatically staged in your 
config directory.
Member

fperez commented Dec 18, 2011

Shouldn't we then instead just fix profile list to show the existing ones from IPython? I imagine something like

ipython profile list

Available profiles in IPython:
- foo
- bar

Available profiles in /path/to/your/config/ipython:
-baz
-zap

Local profiles in the current directory (/path/to/cwd):
- x
- y

Note: you can use any of these by using `ipython profile --profile <name>`; 
if it is one of those within ipython it will be automatically staged in your 
config directory.
@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Dec 18, 2011

Member

Sounds good, see PR #1176 for sample output.

Member

minrk commented Dec 18, 2011

Sounds good, see PR #1176 for sample output.

@fperez

This comment has been minimized.

Show comment
Hide comment
@fperez

fperez Dec 18, 2011

Member

I like #1176 a lot, so I guess the plan will be to merge this and also 1176, right?

Member

fperez commented Dec 18, 2011

I like #1176 a lot, so I guess the plan will be to merge this and also 1176, right?

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Dec 18, 2011

Member

Yes, I think so. Do we want print-statements on the removed magics, or just remove them?

Member

minrk commented Dec 18, 2011

Yes, I think so. Do we want print-statements on the removed magics, or just remove them?

@fperez

This comment has been minimized.

Show comment
Hide comment
@fperez

fperez Dec 18, 2011

Member

Let's do print statements, to help users who might know about them transition more easily.

Member

fperez commented Dec 18, 2011

Let's do print statements, to help users who might know about them transition more easily.

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Dec 18, 2011

Member

Also, searching our docs suggests that we never even mentioned these magics, so there shouldn't be anything to change/update there.

Member

minrk commented Dec 18, 2011

Also, searching our docs suggests that we never even mentioned these magics, so there shouldn't be anything to change/update there.

@fperez

This comment has been minimized.

Show comment
Hide comment
@fperez

fperez Dec 18, 2011

Member

Great, thanks. Just a quick print statement is enough then.

Member

fperez commented Dec 18, 2011

Great, thanks. Just a quick print statement is enough then.

@fperez

This comment has been minimized.

Show comment
Hide comment
@fperez

fperez Dec 19, 2011

Member

@minrk, can you take a shot at this one? I should be done with my talk shortly and will start writing up the release notes and example notebooks. I'd love a hand with this code.

Member

fperez commented Dec 19, 2011

@minrk, can you take a shot at this one? I should be done with my talk shortly and will start writing up the release notes and example notebooks. I'd love a hand with this code.

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Dec 19, 2011

Member

Sure, should I go ahead and merge with the added print statements, or do a quick PR?

Member

minrk commented Dec 19, 2011

Sure, should I go ahead and merge with the added print statements, or do a quick PR?

@fperez

This comment has been minimized.

Show comment
Hide comment
@fperez

fperez Dec 19, 2011

Member

I'm ok with a merge, but if you prefer to keep the PR workflow, I can look at it too.

Member

fperez commented Dec 19, 2011

I'm ok with a merge, but if you prefer to keep the PR workflow, I can look at it too.

@minrk minrk closed this in ac1e66f Dec 19, 2011

@minrk minrk closed this in 2c5f256 Dec 19, 2011

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Dec 19, 2011

Member

merged

Member

minrk commented Dec 19, 2011

merged

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014

Remove %install_default_config and %install_profiles
as they are now controllen with "ipython profile" commands.

closes #1174 and Redhad bug #751146

Signed-off-by: Thomas Spura <thomas.spura@gmail.com>

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014

Merge PR #1174 (remove profile magics)
replaces broken %install_profiles and %install_default_config magics
with simple print statements.

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