Repository: ishepard/pydriller (python)

Issue:
"Now the user needs to change from_commit and to_commit if reversed_order is True" may be misleading
Hi @ishepard ,

I have noticed that during commit https://github.com/ishepard/pydriller/commit/bec86e2fe0c027032e8f66427eb4fd363c65e838 you have swap  ```from_commit``` and ```to_commit``` in the test data for the process metrics to reflect the changes you have done in commit https://github.com/ishepard/pydriller/commit/f84047a2579a4f0b282aebc66a3e6a8d13ae2871.

Although those tests pass, I think it could be misleading telling the user to reverse the order of the two parameters, for two reasons:

1. Different applications already use PyDriller, and I imagine that some of them use the ```reverse_order``` and need to change the code accordingly;

2. In the specific case of the implemented process metrics, the user does not know (and is not assumed to know) that we set up ```reverse_order = True``` in the body of the metrics. When a user calls ```commits_count('pydriller', from_commit='commit_1', to_commit='commit_10')```, he/she expects to analyse the commits in chronological order from ```commit_1``` to ```commit_10```. 
The current version returns the wrong result, and the user has to call ```commits_count('pydriller', from_commit='commit_10', to_commit='commit_1')``` instead, which is IMHO a bit confusing.

Here the alternatives that I propose:

1. Handle the ```reverse_order``` in the constructor of ```RepositoryMining```, by swapping ```from_commit``` and ```to_commit``` in the configuration dict when ```reverse_order == True```, instead of letting the user do so;

2. We can leave everything as it is (but take care of point 1 in the previous list), and I can change the process metrics accordingly, by simply swapping the two variables when calling ```RepositoryMining``` in the body of each metrics.

Let me know what do you think.

Pull request:
Fix order of commit filters
Fix #83 

Patch:
diff --git a/pydriller/utils/conf.py b/pydriller/utils/conf.py
index a7ee3d4..64bec22 100644
--- a/pydriller/utils/conf.py
+++ b/pydriller/utils/conf.py
@@ -81,6 +81,8 @@ class Conf:
             self.set_value("to_commit", None)
             self.set_value("single", single)
 
+        self._check_correct_filters_order()
+
         if self.get('single') is not None:
             if any([self.get('since'),
                     self.get('to'),
@@ -91,6 +93,33 @@ class Conf:
                 raise Exception('You can not specify a single commit with '
                                 'other filters')
 
+    def _check_correct_filters_order(self):
+        """
+        from_commit should come before to_commit when analyzing a repository
+        with the default settings, while they should be swapped when
+        analyzing the repository with reversed_order=True
+        """
+        if self.get('from_commit') and self.get('to_commit'):
+            chronological_order = self._is_commit_before(
+                self.get('git_repo').get_commit(self.get('from_commit')),
+                self.get('git_repo').get_commit(self.get('to_commit')))
+
+            if self.get('reversed_order') and chronological_order:
+                self._swap_commit_fiters()
+            elif not self.get('reversed_order') and not chronological_order:
+                self._swap_commit_fiters()
+
+    def _swap_commit_fiters(self):
+        # reverse from and to commit
+        from_commit = self.get('from_commit')
+        to_commit = self.get('to_commit')
+        self.set_value('from_commit', to_commit)
+        self.set_value('to_commit', from_commit)
+
+    @staticmethod
+    def _is_commit_before(commit_before, commit_after):
+        return commit_before.committer_date < commit_after.committer_date
+
     def check_starting_commit(self):
         """
         Get the starting commit from the 'since', 'from_commit' or 'from_tag'


Test patch:
diff --git a/tests/integration/test_ranges.py b/tests/integration/test_ranges.py
index e3ab613..c557855 100644
--- a/tests/integration/test_ranges.py
+++ b/tests/integration/test_ranges.py
@@ -129,6 +129,13 @@ def test_from_and_to_commit(repository_mining_cc, expected_commits):
     assert len(repository_mining_cc) == expected_commits
 
 
+@pytest.mark.parametrize('from_commit,to_commit,expected_commits', [
+    ('09f6182cef737db02a085e1d018963c7a29bde5a', '6411e3096dd2070438a17b225f44475136e54e3a', 2),
+])
+def test_from_and_to_commit(repository_mining_cc, expected_commits):
+    assert len(repository_mining_cc) == expected_commits
+
+
 # FROM AND TO TAG
 @pytest.mark.parametrize('to_tag,expected_commits', [
     ('v1.4', 3),
diff --git a/tests/integration/test_reverse_order.py b/tests/integration/test_reverse_order.py
index 8069d98..c7854f6 100644
--- a/tests/integration/test_reverse_order.py
+++ b/tests/integration/test_reverse_order.py
@@ -49,3 +49,14 @@ def test_should_visit_descendent_order_with_filters():
     assert lc[0].hash == '1f99848edadfffa903b8ba1286a935f1b92b2845'
     assert lc[1].hash == '09f6182cef737db02a085e1d018963c7a29bde5a'
     assert lc[2].hash == '6411e3096dd2070438a17b225f44475136e54e3a'
+
+
+def test_should_visit_descendent_order_with_filters_reversed():
+    lc = list(RepositoryMining('test-repos/small_repo',
+                               from_commit='6411e3096dd2070438a17b225f44475136e54e3a',
+                               to_commit='1f99848edadfffa903b8ba1286a935f1b92b2845',
+                               reversed_order=True).traverse_commits())
+    assert len(lc) == 3
+    assert lc[0].hash == '1f99848edadfffa903b8ba1286a935f1b92b2845'
+    assert lc[1].hash == '09f6182cef737db02a085e1d018963c7a29bde5a'
+    assert lc[2].hash == '6411e3096dd2070438a17b225f44475136e54e3a'
\ No newline at end of file
diff --git a/tests/metrics/process/test_commits_count.py b/tests/metrics/process/test_commits_count.py
index 1a54796..2057299 100644
--- a/tests/metrics/process/test_commits_count.py
+++ b/tests/metrics/process/test_commits_count.py
@@ -6,6 +6,7 @@ from pydriller.metrics.process.commits_count import CommitsCount
 
 TEST_DATA = [
     ('test-repos/pydriller', 'domain/developer.py', 'fdf671856b260aca058e6595a96a7a0fba05454b', 'ab36bf45859a210b0eae14e17683f31d19eea041', 2),
+    ('test-repos/pydriller', 'domain/developer.py', 'ab36bf45859a210b0eae14e17683f31d19eea041', 'fdf671856b260aca058e6595a96a7a0fba05454b', 2),
     ('test-repos/pydriller', 'domain/developer.py', 'fdf671856b260aca058e6595a96a7a0fba05454b', None, 2),
     ('test-repos/pydriller', 'pydriller/domain/developer.py', None, None, 8),
 ]
diff --git a/tests/test_memory_consumption.py b/tests/test_memory_consumption.py
index 2a8d153..75270fc 100644
--- a/tests/test_memory_consumption.py
+++ b/tests/test_memory_consumption.py
@@ -44,23 +44,36 @@ def test_memory(caplog):
     logging.warning("Starting with metrics...")
     diff_with_metrics, all_commits_with_metrics = mine(2)
 
-    max_values = [max(all_commits_with_nothing), max(all_commits_with_everything), max(all_commits_with_metrics)]
+    max_values = [max(all_commits_with_nothing),
+                  max(all_commits_with_everything),
+                  max(all_commits_with_metrics)]
     logging.warning("Max values are: {}".format(max_values))
 
     minutes_with_everything = (diff_with_everything.seconds % 3600) // 60
     minutes_with_metrics = (diff_with_metrics.seconds % 3600) // 60
 
-    logging.warning("TIME: With nothing: {}:{}:{}, with everything: {}:{}:{}, with metrics: {}:{}:{}".format(
-        diff_with_nothing.seconds // 3600, (diff_with_nothing.seconds % 3600) // 60, diff_with_nothing.seconds % 60,
-        diff_with_everything.seconds // 3600, (diff_with_everything.seconds % 3600) // 60,
+    logging.warning("TIME: With nothing: {}:{}:{} ({} commits/sec), "
+                    "with everything: {}:{}:{}  ({} commits/sec), "
+                    "with metrics: {}:{}:{}  ({} commits/sec)".format(
+        diff_with_nothing.seconds // 3600,
+        (diff_with_nothing.seconds % 3600) // 60,
+        diff_with_nothing.seconds % 60,
+        973 // diff_with_nothing.seconds,
+        diff_with_everything.seconds // 3600,
+        (diff_with_everything.seconds % 3600) // 60,
         diff_with_everything.seconds % 60,
-        diff_with_metrics.seconds // 3600, (diff_with_metrics.seconds % 3600) // 60, diff_with_metrics.seconds % 60
+        973 // diff_with_everything.seconds,
+        diff_with_metrics.seconds // 3600,
+        (diff_with_metrics.seconds % 3600) // 60,
+        diff_with_metrics.seconds % 60,
+        973 // diff_with_metrics.seconds
     ))
 
     if any(val > 250 for val in max_values) or \
             minutes_with_everything >= 1 or \
             minutes_with_metrics >= 7:
-        # if to analyze 1000 commits requires more than 250MB of RAM, more than 1 minute without metrics or
+        # if to analyze 1000 commits requires more than 250MB of RAM,
+        # more than 1 minute without metrics or
         # 7 minutes with metrics, print it on the Slack channel
         logs_and_post_on_slack(diff_with_nothing, all_commits_with_nothing,
                                diff_with_everything, all_commits_with_everything,