eyeonus / EDDBlink

(No longer maintained) A plugin for TradeDangerous to update market data using EDDB's api files.
GNU Lesser General Public License v3.0
2 stars 0 forks source link

New EDDBlink parameter #5

Closed MarkAusten closed 6 years ago

MarkAusten commented 6 years ago

EDDBLink Enhancement Request.

Please would add a new parameter to the eddblink plugin that allows the display of the percentage complete update to be limited to the specified number.

For example:

--percentage=0.1 would only update the display when the percentage changes by 0.1. i.e. 0.1, 0.2, 0.3 .... --percentage=1 would only update the display when the percentage changes by 1. i.e. 1, 2, 3 .... --percentage=-1 would not display the percentage complete.

the --percentage is just a suggestion for the parameter name, a better one with the word 'increment' in it would probably be better.

Reason for request.

The TDHelper uses a worker thread to issue the command to python and as a callback for displaying the output from the plugin, but the update displays on a separate line in the TDHelper output window, not overwriting the existing line as happens in the command window. Updating as often as it does right now a) bollixes the display and b) really, really slows down the execution time.

aadler commented 6 years ago

I'm not at home and cannot try it, but if you pass the variable as "percentage" shouldn't it be something along the lines of:


progress += 1
if percentage > 0:
    chunk = total // (percentage * 100)
    if progress % chunk = 0:
        print("\rProgress: (" + str(progress) + "/" + str(total) + ") " + str(round(progress / total * 100, 2)) + "%    ", 
MarkAusten commented 6 years ago

Something like that except you would not recalculate the chunk value in the loop, that should be done before the loop gets started.

aadler commented 6 years ago

Of course.

On Thu, Jun 21, 2018 at 2:10 PM MarkAusten notifications@github.com wrote:

Something like that except you would not recalculate the chunk value in the loop, that should be done before the loop gets started.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/eyeonus/EDDBlink/issues/5#issuecomment-399194814, or mute the thread https://github.com/notifications/unsubscribe-auth/AFVk8eg1wjLTcC5p7Q9dWgzhm02ysEqOks5t--F8gaJpZM4UyKIr .

-- Sent from Gmail Mobile

MarkAusten commented 6 years ago

At the risk of being called a pedant, I think the code should be:

if displayInterval > 0:
    percent = round(progress / total * 100 / 2)
    if percent % displayInterval == 0:
        print("\rProgress: (" + str(progress) + "/" + str(total) + ") " + str(percent) + "%    ", end = "/r")

where displayInterval is the parameter.

This will update the display in exact multiples of the displayInterval whereas @aadler's code may well give fractional displays in the case where chunk is not an integer factor of total.

Since this code appears three times in the code and is not a trivial change, it might be a candidate for a method.

MarkAusten commented 6 years ago

An alternative would be to use the progress class (progress.py) from the trade dangerous code.

eyeonus commented 6 years ago

That should be ', 2)', not '/ 2)'.

Problem is, TD doesn't allow passing parameters like that. It only allows passing an option name.

In order to have multiple values for "displayInterval", there would have to be multiple options, i.e., per_100th, per_10th, per_1, per_off

With that in mind, I'm just going to add a progress off option. I'll look into using the progress class.

MarkAusten commented 6 years ago

Having implemented the progress bar as a test yesterday, I'd say that it works quite well but turning all progress off would only be needed in the case where you were implementing unattended operation. Some of the updates take a long time and need some sort of progress so that you can see that the routine hasn't stalled.

I set the width of the bar to 100 and tried it out in the command window and in TDHelper and whilst not as informative as your current progress, it worked well. The performance also increased in TDHelper, which is not at all surprising.

I think we may be getting into the territory of having a parameter that turns on the progress bar otherwise the current progress messages. Having used both forms now I find that my initial suggestion is a) not as good as I thought it would be and b) a little over the top as I don't think there is really a need for the different interval rates which is good for the reasons you have already stated. The progress bar is probably the better option for TDHelper.

eyeonus commented 6 years ago

It sounds to me like you've already done most of the work, why don't you submit your code as a pr?

I was thinking about adjusting the progress bar to include the percentage at least, which would be about midway between my progress and the current bar in terms of informative-ness.

MarkAusten commented 6 years ago

I'll need to tidy my code up a bit before doing that, but I will look at doing that this afternoon. Expect a PR shortly.

eyeonus commented 6 years ago

"progbar" option added. Also added to eye-TD plugin version.