-
Notifications
You must be signed in to change notification settings - Fork 160
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
Various fixes and minor adjustments #257
Merged
Merged
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
b03aad8
Optimize diff: Don't deepcopy data
vidartf bd4e6e8
Cleanup
vidartf 89bb72f
Some profiling tools
vidartf c76c2e4
Typo in comment.
d658d34
Bugfix in pointer regex.
44b5930
Minor adjustments
416c0cb
A couple of bugfixes in merge algorithm.
6b2e4d5
Remove old fixme comments.
028f0e1
Improve drag workaround
vidartf 4880e52
Use `re.IGNORECASE` for diffing regexps
vidartf File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
|
||
"""Tools for profiling diff/merge performance. | ||
|
||
Sometimes diffing times can become unacceptably long. | ||
Then the tools in this module can help you profile and figure out | ||
where the time is spent. For a more rigorous profiling, consider using | ||
cPython, but for initial considerations these should be helpful. | ||
|
||
Typical profiling usage: | ||
Add some statements like | ||
from nbdime.profiling import timer | ||
with timer.time('Key to identify this segment'): | ||
<code to time> | ||
|
||
Then, launch `python -m nbdime.profiling notebookA.ipynb notebookB.ipynb`. | ||
|
||
Example case: | ||
Output diffing is slow. By cPython/timer, the time spent has been narrowed | ||
to comparison of MIME types (`_compare_mimedata()`). To figure out which | ||
MIME type is taking all the time, the following is added to that function: | ||
|
||
with timer.time(mimetype): | ||
<existing function body> | ||
|
||
The output becomes: | ||
|
||
Key Calls Time | ||
---------- ------- ------------ | ||
text/html 6 74.9686 | ||
text/plain 12 0.000500202 | ||
image/png 5 0 | ||
|
||
From this we can see that the HTML comparison is really blowing up! | ||
Further inspection reveals that there are two html outputs in each | ||
notebook that contain an embedded base64 image. These caused the text | ||
diff to blow up. | ||
|
||
""" | ||
|
||
import time | ||
import contextlib | ||
from tabulate import tabulate | ||
|
||
|
||
def _sort_time(value): | ||
time = value[1]['time'] | ||
return -time | ||
|
||
|
||
class TimePaths(object): | ||
def __init__(self, verbose=False, enabled=True): | ||
self.verbose = verbose | ||
self.map = {} | ||
self.enabled = enabled | ||
|
||
@contextlib.contextmanager | ||
def time(self, key): | ||
start = time.time() | ||
yield | ||
end = time.time() | ||
secs = end - start | ||
if not self.enabled: | ||
return | ||
if key in self.map: | ||
self.map[key]['time'] += secs | ||
self.map[key]['calls'] += 1 | ||
else: | ||
self.map[key] = dict(time=secs, calls=1) | ||
|
||
@contextlib.contextmanager | ||
def enable(self): | ||
old = self.enabled | ||
self.enabled = True | ||
yield | ||
self.enabled = old | ||
|
||
@contextlib.contextmanager | ||
def disable(self): | ||
old = self.enabled | ||
self.enabled = False | ||
yield | ||
self.enabled = old | ||
|
||
def __str__(self): | ||
# First, sort by path | ||
items = sorted(self.map.items(), key=_sort_time) | ||
lines = [] | ||
for key, data in items: | ||
time = data['time'] | ||
calls = data['calls'] | ||
lines.append((key, calls, time)) | ||
return tabulate(lines, headers=['Key', 'Calls', 'Time']) | ||
|
||
|
||
timer = TimePaths(enabled=False) | ||
|
||
|
||
def profile_diff_paths(args=None): | ||
import nbdime.nbdiffapp | ||
import nbdime.profiling | ||
nbdime.nbdiffapp.main(args) | ||
data = str(nbdime.profiling.timer) | ||
print(data) | ||
|
||
|
||
if __name__ == "__main__": | ||
profile_diff_paths() |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These can be simplified by adding
, re.IGNORECASE)
instead of explicit casingThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed.