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

History access #824

Merged
merged 7 commits into from Oct 14, 2011
Merged

History access #824

merged 7 commits into from Oct 14, 2011

Conversation

takluyver
Copy link
Member

This separates out an HistoryAccessor class, which can be used to read the history db without initialising an IPython shell. HistoryManager becomes a subclass of that, adding in the methods to write to the database.

@fperez
Copy link
Member

fperez commented Sep 30, 2011

Hey Thomas, it looks like conflicts slipped in from recent merges. Could you rebase before we can have a look? Thx!

@takluyver
Copy link
Member Author

Done, it was only one small conflict.

output_hist_reprs = Dict()

class HistoryAccessor(Configurable):
"""Access the history database without adding to it. For use by standalone
Copy link
Member

Choose a reason for hiding this comment

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

Let's try to keep, here and elsewhere, to the format for docstrings (just like git commit messages) of 'one line summary, blank line, more verbose description). Tooltips that use the one line summary look bad if it's broken up.

@fperez
Copy link
Member

fperez commented Oct 1, 2011

Otherwise looks good. It seems to be mostly refactoring, which means it should be pretty safe. But since I see there's a new method (for range), I'd like to see at least a test or two for that guy in isolation. The better we can make our test coverage...

@takluyver
Copy link
Member Author

OK, I've tweaked that docstring. I don't think I've added any really new methods - HistoryAccessor has a get_range method which requires a positive session number specified. HistoryManager overrides that with a version which has a default value for session (i.e. the current one), and allows negative numbers to count back from that.

@fperez
Copy link
Member

fperez commented Oct 8, 2011

No, what I meant is that now there's a bit of logic that's nicely isolated in the private _get_range_session. I saw that method as a good candidate to add a small test or two that would improve our coverage.

I hope soon we'll start turning on test coverage reports in the test suite, so we can begin tracking our progress on that front. In the meantime, every little bit we can do will help. What do you think?

@takluyver
Copy link
Member Author

Most of it should be indirectly covered by this call and this call, but I'll add some more specific tests for it.

@takluyver
Copy link
Member Author

Added some tests.

@minrk
Copy link
Member

minrk commented Oct 14, 2011

Is this ready for merge? It sounds like review has been covered, assuming tests pass.

@takluyver
Copy link
Member Author

As far as I know, it's ready to go.

@minrk
Copy link
Member

minrk commented Oct 14, 2011

Okay, then I say go for it. I think writing some good examples for turning sessions into scripts etc. with the new Accessor would be great, since it's fairly easy, but not immediately obvious.

@takluyver
Copy link
Member Author

Actually, just thinking about it, I'd like to add a slightly nicer interface to load history for a particular profile. Do we have a function that will get the folder (as in, $IPYTHONDIR/profile_whatever) for a given profile name?

@minrk
Copy link
Member

minrk commented Oct 14, 2011

We have ProfileDir objects for managing/creating/locating profile directories, so you can do:

from IPython.core.profiledir import ProfileDir
pd = ProfileDir.find_profile_dir_by_name(get_ipython_dir(), 'foo')
path = pd.location

Perhaps adding this to utils.path would be useful:

def locate_profile(profile='default'):
    from IPython.core.profiledir import ProfileDir, ProfileDirError
    try:
        pd = ProfileDir.find_profile_dir_by_name(get_ipython_dir(), profile)
    except ProfileDirError:
        # IOError makes more sense when people are expecting a path
        raise IOError("Couldn't find profile %r" % profile)
    return pd.location

@takluyver
Copy link
Member Author

Thanks Min, that's just the ticket.

@takluyver takluyver merged commit a355a9f into ipython:master Oct 14, 2011
@takluyver
Copy link
Member Author

OK, I've made the API improvement I wanted, added an example script (docs/examples/core/ipython-extract-history.py), merged and checked the tests. Hopefully post-0.12 someone will be inspired to build a nice history viewer/dumper.

@minrk
Copy link
Member

minrk commented Oct 14, 2011

Thanks!

Yes, that would be great. All of the %log, etc. tools should probably use this as well.

@takluyver
Copy link
Member Author

Magics like %hist, %macro, %save and so on already do. The %log* machinery writes cells directly to a text file.

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.

None yet

3 participants