Skip to content
This repository was archived by the owner on Mar 13, 2026. It is now read-only.

ENH: query price presentation (uses pyfiglet fonts)#71

Merged
tworec merged 1 commit into
googleapis:masterfrom
RTBHOUSE:ENH-figlet-price
Jul 12, 2017
Merged

ENH: query price presentation (uses pyfiglet fonts)#71
tworec merged 1 commit into
googleapis:masterfrom
RTBHOUSE:ENH-figlet-price

Conversation

@tworec

@tworec tworec commented Jul 10, 2017

Copy link
Copy Markdown
Contributor

Hi,
to keep up the momentum I'm sharing my fancy query price print outs.
Feel free to abandon this if you do not like it.
At RTBHOUSE we are using it with pleasure :)

Feature:
Query price is printed with ascii art font. Font gets larger as price increases.

Sample outputs

  • below $0,1 (plain text)
Processed: 10.3 Gb
Price: 0.05 $
Retrieving results...
  • between $0,1 - $0,5 (first level of ascii font)
Processed: 31.3 GB
Price: 
 ||_    __         __ 
(||    /  \    /| |_  
_||)   \__/.    | __) 
 ||

Retrieving results...
  • between $0.5 - $1
     __,--,_.     ___      ______   _____  
    /       |    / _ \    |____  | | ____| 
   |   (----`   | | | |       / /  | |__   
    \   \       | | | |      / /   |___ \  
.----)   |      | |_| |  __ / /     ___) | 
|_    __/        \___/  (__)_/     |____/  
  '--'                                     
  • between $1 - $2
Processed: 219.2 Gb
Price: 
 .----------------.  .----------------.  .----------------.  .----------------.   
| .--------------. || .--------------. || .--------------. || .--------------. |  
| |     __       | || |              | || |     ____     | || |   _______    | |  
| |    /  |      | || |              | || |   .'    '.   | || |  |  ___  |   | |  
| |    `| |      | || |              | || |  |  .--.  |  | || |  |_/  / /    | |  
| |     | |      | || |              | || |  | |    | |  | || |      / /     | |  
| |    _| |_     | || |      _       | || |  |  `--'  |  | || |     / /      | |  
| |   |_____|    | || |     (_)      | || |   '.____.'   | || |    /_/       | |  
| |              | || |              | || |              | || |              | |  
| '--------------' || '--------------' || '--------------' || '--------------' |  
 '----------------'  '----------------'  '----------------'  '----------------'   

Retrieving results...
  • between $2 - $5
Processed: 501.9 Gb
Price: 
                                                                        
                                                                        
       .            .--~*teu.                   xeee      cuuu....uK    
       E           dF     988Nx                d888R      888888888     
    .x+E:..       d888b   `8888>              d8888R      8*888**"      
  u8~  E  `b.     ?8888>  98888F             @ 8888R      >  .....      
 t8E   E d888>     "**"  x88888~           .P  8888R      Lz"  ^888Nu   
 88N.  E'8888~          d8888*`           :F   8888R      F     '8888k  
 88888b&.`""`         z8**"`   :         x"    8888R      ..     88888> 
 '88888888e.        :?.....  ..F    .   d8eeeee88888eer  @888L   88888  
   "*8888888N      <""888888888~  .@8c         8888R    '8888F   8888F  
  uu. ^8*8888E     8:  "888888*  '%888"        8888R     %8F"   d888"   
 @888L E `"88E     ""    "**"`     ^*       "*%%%%%%**~   ^"===*%"`     
'8888~ E   98~                                                          
 `*.   E  .*"                                                           
   `~==R=~`                                                             
  

Retrieving results...
  • above $5
       $$$$$                                                                             
       $:::$                                                                             
   $$$$$:::$$$$$$              66666666                000000000     77777777777777777777
 $$::::::::::::::$            6::::::6               00:::::::::00   7::::::::::::::::::7
$:::::$$$$$$$::::$           6::::::6              00:::::::::::::00 7::::::::::::::::::7
$::::$       $$$$$          6::::::6              0:::::::000:::::::0777777777777:::::::7
$::::$                     6::::::6               0::::::0   0::::::0           7::::::7 
$::::$                    6::::::6                0:::::0     0:::::0          7::::::7  
$:::::$$$$$$$$$          6::::::6                 0:::::0     0:::::0         7::::::7   
 $$::::::::::::$$       6::::::::66666            0:::::0 000 0:::::0        7::::::7    
   $$$$$$$$$:::::$     6::::::::::::::66          0:::::0 000 0:::::0       7::::::7     
            $::::$     6::::::66666:::::6         0:::::0     0:::::0      7::::::7      
            $::::$     6:::::6     6:::::6        0:::::0     0:::::0     7::::::7       
$$$$$       $::::$     6:::::6     6:::::6        0::::::0   0::::::0    7::::::7        
$::::$$$$$$$:::::$     6::::::66666::::::6        0:::::::000:::::::0   7::::::7         
$::::::::::::::$$       66:::::::::::::66  ......  00:::::::::::::00   7::::::7          
 $$$$$$:::$$$$$           66:::::::::66    .::::.    00:::::::::00    7::::::7           
      $:::$                 666666666      ......      000000000     77777777            
      $$$$$                                                                              

@tworec tworec force-pushed the ENH-figlet-price branch from 0c586be to 66ecb06 Compare July 10, 2017 12:47
@codecov-io

codecov-io commented Jul 10, 2017

Copy link
Copy Markdown

Codecov Report

Merging #71 into master will decrease coverage by 45.82%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #71       +/-   ##
===========================================
- Coverage   73.99%   28.17%   -45.83%     
===========================================
  Files           4        4               
  Lines        1542     1544        +2     
===========================================
- Hits         1141      435      -706     
- Misses        401     1109      +708
Impacted Files Coverage Δ
pandas_gbq/gbq.py 19.2% <0%> (-58.3%) ⬇️
pandas_gbq/tests/test_gbq.py 27.94% <0%> (-54.53%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 269685c...8982378. Read the comment docs.

@tworec tworec force-pushed the ENH-figlet-price branch from 66ecb06 to 6e81cef Compare July 10, 2017 13:17
Comment thread pandas_gbq/gbq.py Outdated
import sys

import numpy as np
from pyfiglet import Figlet

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would need to be added as a dependency in setup.py as well.

Comment thread pandas_gbq/gbq.py Outdated

def query_price_for(self, bytes_num):
figlet = None
price = bytes_num * self.query_price_for_TB / 2**40

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's reasonable to put the price estimate in, but note that it won't always apply since there is also a fixed price plan.

I'm not sure about the fancy text rendering. It's fun, but is it worth the extra dependency?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, I agree re the extra dependency. Maaaybe as a soft dependency

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My personal preference is to remove the pyfiglet font and just report the pricing in the default font.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, so if there is no hard veto's (@jreback?!) i'll try to push this forward without figlet fonts

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thus shouldn't be a hard dep

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i can make it an option i guess but not really sure it adds value

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i -> you

@parthea parthea left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @tworec ! I have a couple minor observations.

Comment thread pandas_gbq/gbq.py Outdated

def query_price_for(self, bytes_num):
figlet = None
price = bytes_num * self.query_price_for_TB / 2**40

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My personal preference is to remove the pyfiglet font and just report the pricing in the default font.

Comment thread pandas_gbq/gbq.py Outdated
num /= 1024.0
return fmt % (num, 'Y', suffix)

def query_price_for(self, bytes_num):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we rename query_price_for to something more descriptive like query_price_from_byte_count or similar?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also add a unit test for query_price_for so that our code coverage doesn't decrease?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@tworec tworec force-pushed the ENH-figlet-price branch 2 times, most recently from 8ee1efd to fd5e0e6 Compare July 11, 2017 17:19
@tworec

tworec commented Jul 11, 2017

Copy link
Copy Markdown
Contributor Author

now it's clean and easy! please revie

@parthea parthea left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @tworec !

My only suggestion would be to include '$' before the price and also include the currency but I'm ok with the current version.

@tworec tworec force-pushed the ENH-figlet-price branch from fd5e0e6 to 8982378 Compare July 12, 2017 09:53
@tworec

tworec commented Jul 12, 2017

Copy link
Copy Markdown
Contributor Author

@parthea, good point, thx
now price is well formatted with '$' prefix and USD suffix

@tworec

tworec commented Jul 12, 2017

Copy link
Copy Markdown
Contributor Author

Sample output is:

Requesting query... ok.
Job ID: job_XXXxXxXX0xxXxX00xxx00X_XxXx
Query running...
Query done.
Processed: 18.9 GB
Standard price: $0.09 USD

Retrieving results...
Got 1 rows.

Total time taken 4.08 s.
Finished at 2017-07-12 15:17:38.

@tworec tworec merged commit 964a19b into googleapis:master Jul 12, 2017
@tworec tworec deleted the ENH-figlet-price branch July 12, 2017 13:26
@jreback

jreback commented Jul 12, 2017

Copy link
Copy Markdown
Contributor

generally wait for comments on PRs before merging pls

@tworec

tworec commented Jul 13, 2017

Copy link
Copy Markdown
Contributor Author

Generally, ok.

But in this specific case, there were two approval comments, and since my further changes does only modified string literal, which pass my local manual checks and Travis builds as well, I've decided to go forward and merge them.

Is there any problem with merged code?

@jreback

jreback commented Jul 13, 2017

Copy link
Copy Markdown
Contributor

@tworec not a problem.

@tworec

tworec commented Jul 13, 2017

Copy link
Copy Markdown
Contributor Author

ok :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants