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

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

Merged
merged 1 commit into from
Jul 12, 2017

Conversation

tworec
Copy link
Contributor

@tworec tworec commented Jul 10, 2017

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            
      $$$$$                                                                              

@codecov-io
Copy link

codecov-io commented Jul 10, 2017

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.

@@ -7,6 +7,9 @@
import sys

import numpy as np
from pyfiglet import Figlet
Copy link
Collaborator

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.

@@ -421,6 +439,17 @@ def sizeof_fmt(num, suffix='B'):
num /= 1024.0
return fmt % (num, 'Y', suffix)

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

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
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
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
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
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
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
Contributor

Choose a reason for hiding this comment

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

i -> you

Copy link
Contributor

@parthea parthea left a comment

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.

@@ -421,6 +439,17 @@ def sizeof_fmt(num, suffix='B'):
num /= 1024.0
return fmt % (num, 'Y', suffix)

def query_price_for(self, bytes_num):
figlet = None
price = bytes_num * self.query_price_for_TB / 2**40
Copy link
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.

@@ -421,6 +439,17 @@ def sizeof_fmt(num, suffix='B'):
num /= 1024.0
return fmt % (num, 'Y', suffix)

def query_price_for(self, bytes_num):
Copy link
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
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
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
Copy link
Contributor Author

tworec commented Jul 11, 2017

now it's clean and easy! please revie

Copy link
Contributor

@parthea parthea left a comment

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
Copy link
Contributor Author

tworec commented Jul 12, 2017

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

@tworec
Copy link
Contributor Author

tworec commented Jul 12, 2017

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
Copy link
Contributor

jreback commented Jul 12, 2017

generally wait for comments on PRs before merging pls

@tworec
Copy link
Contributor Author

tworec commented Jul 13, 2017

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
Copy link
Contributor

jreback commented Jul 13, 2017

@tworec not a problem.

@tworec
Copy link
Contributor Author

tworec commented Jul 13, 2017

ok :)

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

Successfully merging this pull request may close these issues.

6 participants