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

self.avg vs. (self.elapsed/self.index) #34

Closed
clime opened this issue Nov 1, 2016 · 9 comments
Closed

self.avg vs. (self.elapsed/self.index) #34

clime opened this issue Nov 1, 2016 · 9 comments

Comments

@clime
Copy link

clime commented Nov 1, 2016

Shouldn't self.avg be (roughly) equal to self.elapsed/self.index? In my test (file uploading) it comes out differently:

1.578591763973236e-08 (self.avg) vs. 7.149365691934405e-08 (self.elapsed/self.index)

Eta is then not correct (less than actual).

Thanks!

@praiskup
Copy link

praiskup commented Nov 1, 2016

@praiskup
Copy link

praiskup commented Nov 1, 2016

@clime, aka #24. Have you tried the latest git head of this project? Last time had a look it did not work.

@clime
Copy link
Author

clime commented Nov 1, 2016

Actually, I am sorry, this is not a bug against upstream (i tested with 1.2-7.fc24). With upstream ETA works correctly.

self.avg gives very high values (~15728944.785901014) and that's likely because the meaning of it has changed.

It is no longer: avg simple moving average time per item (in seconds) (as stated in docs) but instead it seems to be: items per second.

So probably just README.md update is needed and this can be closed. Thx.

@praiskup
Copy link

praiskup commented Nov 1, 2016

Ouch, you claim semantics of API changed?

@praiskup
Copy link

praiskup commented Nov 1, 2016

FWIW, according to Fedora bug:
http://pkgs.fedoraproject.org/cgit/rpms/python-progress.git/commit/?id=2e851c8ab6e759a75d04d20d763e01e5eb06692f
@clime, can you check the Rawhide is fixed for you? I could submit updates to stable Fedora too.

@clime
Copy link
Author

clime commented Nov 2, 2016

Yes, Fedora-Rawhide version works.

@verigak
Copy link
Owner

verigak commented Nov 3, 2016

So can I merge PR/35 and close this?

@clime
Copy link
Author

clime commented Nov 3, 2016

Fine by me but that #35 request is unneeded if you return to the original semantics :). Anyway, this can be closed.

@verigak
Copy link
Owner

verigak commented Apr 1, 2017

This should be fixed now

@verigak verigak closed this as completed Apr 1, 2017
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

No branches or pull requests

3 participants