Added optional name field to utils.counter #278

matsavage · 2017-11-03T11:27:07Z

Description

Added a naming variable to utils.counter and chnaged time precision

Related issues or pull requests

Link related issues/pull requests here

Pull Request requirements

Added appropriate unit test functions in the ./mlxtend/*/tests directories
Ran nosetests ./mlxtend -sv and make sure that all unit tests pass
Checked the test coverage by running nosetests ./mlxtend --with-coverage
Checked for style issues by running flake8 ./mlxtend
Added a note about the modification or contribution to the ./docs/sources/CHANGELOG.md file
Modify documentation in the appropriate location under mlxtend/docs/sources/ (optional)
Checked that the Travis-CI build passed at https://travis-ci.org/rasbt/mlxtend

Changed time precision to .2

pep8speaks · 2017-11-03T11:27:08Z

Hello @matsavage! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on November 04, 2017 at 17:42 Hours UTC

coveralls · 2017-11-03T11:39:09Z

Coverage decreased (-0.01%) to 90.896% when pulling 2ad446d on matsavage:patch-3 into a7a8f86 on rasbt:master.

coveralls · 2017-11-03T11:50:15Z

Coverage decreased (-0.01%) to 90.896% when pulling 2ad446d on matsavage:patch-3 into a7a8f86 on rasbt:master.

Updated Changelog and tests

coveralls · 2017-11-03T12:46:09Z

Coverage decreased (-0.01%) to 90.896% when pulling fc64415 on matsavage:patch-3 into a7a8f86 on rasbt:master.

rasbt · 2017-11-03T17:41:51Z

Thanks for the PR. How about adding a precision_format parameter instead of changing the default behavior (or do you have a better name than precision_format in mind?)

E.g., by default, this would be

precision_format='%d', and it could then be flexibly changed to any desired precision like precision_format='%.2f' etc.

coveralls · 2017-11-04T14:13:30Z

Coverage decreased (-0.04%) to 90.873% when pulling e8a204f on matsavage:patch-3 into a7a8f86 on rasbt:master.

coveralls · 2017-11-04T14:15:11Z

Coverage decreased (-0.04%) to 90.873% when pulling 3b5a627 on matsavage:patch-3 into a7a8f86 on rasbt:master.

coveralls · 2017-11-04T14:21:46Z

Coverage decreased (-0.04%) to 90.873% when pulling 3b5a627 on matsavage:patch-3 into a7a8f86 on rasbt:master.

matsavage · 2017-11-04T14:22:38Z

@rasbt Great idea!

I added a kwarg that accepts an int, for the number of decimal places to display, which seems cleaner.

To update the precision string I had to do a little refactoring, so added some methods to return the total time elapsed and the time elapsed in the current iteration.

coveralls · 2017-11-04T14:26:39Z

Coverage decreased (-0.04%) to 90.873% when pulling 3b5a627 on matsavage:patch-3 into a7a8f86 on rasbt:master.

rasbt · 2017-11-04T14:43:51Z

Sounds good! Regarding the iteration_time, I don't think that's currently needed in your implementation though as I don't see it currently used anywhere except iteration_elapsed(), which in turn is not used anywhere else?

matsavage · 2017-11-04T15:05:13Z

It's not used internally, but it seems like a useful thing to have, and goes along with the iteration_count variable

…

On 4 Nov 2017, at 14:43, Sebastian Raschka ***@***.***> wrote: Sounds good! Regarding the iteration_time, I don't think that's currently needed in your implementation though as I don't see it currently used anywhere except iteration_elapsed? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

rasbt · 2017-11-04T15:37:04Z

Thanks for the PR, I really appreciate that! However, I'd prefer to keep the Counter class relatively lean and simple. I.e., not adding not too many bells and whistles (methods) for one-liners.

Instead, I would suggest to add the desired features as an example to the documentation, e.g., see the "Examples" section in the docsctring below:

class Counter(object):

    """Class to display the progress of for-loop iterators.

    Parameters
    ----------
    stderr : bool (default: True)
        Prints output to sys.stderr if True; uses sys.stdout otherwise.
    start_newline : bool (default: True)
        Prepends a new line to the counter, which prevents overwriting counters
        if multiple counters are printed in succession.
    precision: int (default: 0)
        Sets the precison of the displayed iteration time.
    name : string (default: None)
        Prepends the specified name before the counter to allow distinguishing
        between multiple counters.

    Attributes
    ----------
    curr_iter : int
        The current iteration.
    start_time : float
        The system's time in seconds when the Counter was initialized.
    end_time : float
        The system's time in seconds when the Counter was last updated.

    Examples
    --------
    >>> cnt = Counter()
    >>> for i in range(20):
    ...     # do some computation
    ...     time.sleep(0.1)
    ...     cnt.update()

    20 iter | 2 sec
    >>> print('The counter was initialized.'
              ' %d seconds ago.' % (time.time() - cnt.start_time))
    The counter was initialized 2 seconds ago
    >>> print('The counter was last updated'
              ' %d seconds ago.' % (time.time() - cnt.end_time))
    The counter was last updated 0 seconds ago.

    """
    def __init__(self, stderr=False, start_newline=True, precision=0,
                 name=None):
        if stderr:
            self.stream = sys.stderr
        else:
            self.stream = sys.stdout
        if isinstance(precision, int):
            self.precision = '%%.%df' % precision
        else:
            self.precision = '%d'
        self.name = name
        self.start_time = time.time()
        self.curr_iter = 0
        if start_newline:
            self.stream.write('\n')

    def update(self):
        """Print current iteration and time elapsed."""
        self.curr_iter += 1

        self.end_time = time.time()

        out = '%d iter | %s sec' % (self.curr_iter,
                                    self.precision % (self.end_time
                                                      - self.start_time))
        if self.name is None:
            self.stream.write('\r%s' % out)
        else:
            self.stream.write('\r %s: %s' % (self.name, out))
        self.stream.flush()

matsavage · 2017-11-04T15:54:00Z

@rasbt That makes much more sense, I got the idea of wanting to keep things lean from the class as it currently stands, which was why I didn't include the precision kwarg on my first iteration.

Good idea on including that as an example, much simpler than my implementation. Sorry about so much back and forth about such a simple change, I'm new to github and contributing changes.

coveralls · 2017-11-04T16:10:12Z

Coverage decreased (-0.03%) to 90.884% when pulling 91ec700 on matsavage:patch-3 into a7a8f86 on rasbt:master.

rasbt · 2017-11-04T16:14:20Z

No worries, we've been all new to GitHub at some point :). Let me make a few more inline comments ...

rasbt · 2017-11-04T16:17:00Z

mlxtend/utils/counter.py

@@ -21,20 +21,49 @@ class Counter(object):
    start_newline : bool (default: True)
        Prepends a new line to the counter, which prevents overwriting counters
        if multiple counters are printed in succession.
+    precision: int (default: 0)
+        Sets the precison of the displayed iteration time.


There's a little typo and it should be "precision" not "precison". However, I suggest changing the short description altogether ... E.g., the number 1234567.89 has a precision of 9, so it would be more "precise" (pun intended :P) to refer to it as:

Sets the number of decimal places when displaying the time elapsed in seconds.

Haha, very good, change made

coveralls · 2017-11-04T16:34:32Z

Coverage decreased (-0.03%) to 90.884% when pulling 2c7d5bb on matsavage:patch-3 into a7a8f86 on rasbt:master.

coveralls · 2017-11-04T16:39:56Z

Coverage decreased (-0.03%) to 90.884% when pulling 2c7d5bb on matsavage:patch-3 into a7a8f86 on rasbt:master.

coveralls · 2017-11-04T17:37:21Z

Coverage decreased (-0.03%) to 90.884% when pulling beb4a38 on matsavage:patch-3 into a7a8f86 on rasbt:master.

coveralls · 2017-11-04T17:53:51Z

Coverage decreased (-0.03%) to 90.884% when pulling f81e656 on matsavage:patch-3 into a7a8f86 on rasbt:master.

rasbt · 2017-11-04T22:48:50Z

Looks good now, thanks! Will merge it and take care of the docs.

Update counter.py

965eba7

Changed time precision to .2

Update counter.py

2ad446d

matsavage added 2 commits November 3, 2017 11:46

Added name test

a8c838a

Update CHANGELOG.md

e7fd77d

matsavage added 3 commits November 3, 2017 11:54

Merge pull request #1 from matsavage/master

0e59677

Updated Changelog and tests

Update counter.py

9f280b2

Update counter.py

67a3201

matsavage changed the title ~~Added optional name field to counter~~ Added optional name field to utils.counter Nov 3, 2017

Update test_counter.py

fc64415

matsavage added 3 commits November 4, 2017 14:02

Update counter.py

45b38f2

Update CHANGELOG.md

e49e700

Update CHANGELOG.md

e8a204f

Update counter.py

3b5a627

Update counter.py

694e260

Update counter.py

91ec700

rasbt requested changes Nov 4, 2017

View reviewed changes

matsavage added 2 commits November 4, 2017 16:22

Update counter.py

312c5d5

Update CHANGELOG.md

2c7d5bb

Update counter.py

beb4a38

Update counter.py

f81e656

rasbt merged commit c2abce5 into rasbt:master Nov 4, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added optional name field to utils.counter #278

Added optional name field to utils.counter #278

matsavage commented Nov 3, 2017 •

edited

Loading

pep8speaks commented Nov 3, 2017 •

edited

Loading

coveralls commented Nov 3, 2017

coveralls commented Nov 3, 2017

coveralls commented Nov 3, 2017

rasbt commented Nov 3, 2017 •

edited

Loading

coveralls commented Nov 4, 2017

coveralls commented Nov 4, 2017

coveralls commented Nov 4, 2017

matsavage commented Nov 4, 2017

coveralls commented Nov 4, 2017

rasbt commented Nov 4, 2017 •

edited

Loading

matsavage commented Nov 4, 2017 via email

rasbt commented Nov 4, 2017

matsavage commented Nov 4, 2017

coveralls commented Nov 4, 2017

rasbt commented Nov 4, 2017

rasbt Nov 4, 2017

matsavage Nov 4, 2017

coveralls commented Nov 4, 2017

coveralls commented Nov 4, 2017

coveralls commented Nov 4, 2017

coveralls commented Nov 4, 2017

rasbt commented Nov 4, 2017

Added optional name field to utils.counter #278

Added optional name field to utils.counter #278

Conversation

matsavage commented Nov 3, 2017 • edited Loading

Description

Related issues or pull requests

Pull Request requirements

pep8speaks commented Nov 3, 2017 • edited Loading

Comment last updated on November 04, 2017 at 17:42 Hours UTC

coveralls commented Nov 3, 2017

coveralls commented Nov 3, 2017

coveralls commented Nov 3, 2017

rasbt commented Nov 3, 2017 • edited Loading

coveralls commented Nov 4, 2017

coveralls commented Nov 4, 2017

coveralls commented Nov 4, 2017

matsavage commented Nov 4, 2017

coveralls commented Nov 4, 2017

rasbt commented Nov 4, 2017 • edited Loading

matsavage commented Nov 4, 2017 via email

rasbt commented Nov 4, 2017

matsavage commented Nov 4, 2017

coveralls commented Nov 4, 2017

rasbt commented Nov 4, 2017

rasbt Nov 4, 2017

Choose a reason for hiding this comment

matsavage Nov 4, 2017

Choose a reason for hiding this comment

coveralls commented Nov 4, 2017

coveralls commented Nov 4, 2017

coveralls commented Nov 4, 2017

coveralls commented Nov 4, 2017

rasbt commented Nov 4, 2017

matsavage commented Nov 3, 2017 •

edited

Loading

pep8speaks commented Nov 3, 2017 •

edited

Loading

rasbt commented Nov 3, 2017 •

edited

Loading

rasbt commented Nov 4, 2017 •

edited

Loading