Velocity Reviews

Velocity Reviews (http://www.velocityreviews.com/forums/index.php)
-   Python (http://www.velocityreviews.com/forums/f43-python.html)
-   -   Re: Confused compare function :) (http://www.velocityreviews.com/forums/t955179-re-confused-compare-function.html)

Bruno Dupuis 12-06-2012 12:19 AM

Re: Confused compare function :)
 
On Wed, Dec 05, 2012 at 11:50:49PM +0100, Anatoli Hristov wrote:
> I'm confused again with a compare update function. The problem is that
> my function does not work at all and I don't get it where it comes
> from.
>
> in my DB I have total of 754 products. when I run the function is says:
> Total updated: 754
> Total not found with in the distributor: 747
> I just don't get it, can you find my mistake ?
>
> Thanks in advance
>
> def Change_price():
> total = 0
> tnf = 0
> for row in DB: # DB is mySQL DB, logically I get out 1 SKU and I
> compare it with next loop
> isku = row["sku"]
> isku = isku.lower()
> iprice = row["price"]
> iprice = int(iprice)
> found = 0
> try:
> for x in PRICELIST:# here is my next loop in a CSV file
> which is allready in a list PRICELIST
> try:
> dprice = x[6]
> dprice = dprice.replace(",",".") # As in the
> PRICELIST the prices are with commas I replace the comma as python
> request it
> dprice = float(dprice)
> newprice = round(dprice)*1.10
> dsku = x[4]
> dsku = dsku.lower()
> stock = int(x[7])
> if isku == dsku and newprice < int(iprice):# If
> found the coresponded SKU and the price is higher than the one in the
> CSV I update the price
> print dsku, x[6], dprice, newprice
> Update_SQL(newprice, isku)# goes to the SQL Update
> print isku, newprice
> if isku == dsku:# Just a check to see if it works
> print "Found %s" %dsku
> found = 1
> else:
> found = 0
> except IndexError:
> pass
> except ValueError:
> pass
> except TypeError:
> pass
> except IndexError:
> pass
> if found == 1:
> print "%s This is match" % isku
> if found == 0:
> print "%s Not found" % isku
> tnf = tnf +1
> total = total +1
> print "Total updated: %s" % total
> print"Total not found with in the distributor: %s" % tnf


I tried, I swear I did try, I didn't understand the whole algorithm of the
function. However, in a first sight, I find it way to deeply nested.
def ... for ... try ... for ... if ... if. Can't you split it in several
function, or in methods of a callable class? Somtimes it's finally much
more clear and the errors become obvious. Another advice: never ever

except XXXError:
pass

at least log, or count, or warn, or anything, but don't pass. I bet your
missing products have disapeared into those black holes.

mmmh, now, i see that you set found to 1 only if newprice <int(iprice)...
new_price is a float (newprice = round(dprice)*1.10) that you compare with
an int? is that correct? seems strangee to me.

--
Bruno Dupuis

Steven D'Aprano 12-06-2012 12:42 AM

Re: Confused compare function :)
 
On Thu, 06 Dec 2012 01:19:58 +0100, Bruno Dupuis wrote:

> I tried, I swear I did try, I didn't understand the whole algorithm of
> the function. However, in a first sight, I find it way to deeply nested.


Yes!

But basically, the code seems to run a pair of nested for-loops:

for SKU in database:
for SKU in csv file:
if the two SKUs match:
compare their prices and update the database



> def ... for ... try ... for ... if ... if.


You missed a second try.

> Can't you split it in several
> function, or in methods of a callable class? Somtimes it's finally much
> more clear and the errors become obvious. Another advice: never ever
>
> except XXXError:
> pass
>
> at least log, or count, or warn, or anything, but don't pass. I bet your
> missing products have disapeared into those black holes.


I think that "never" is too strong, but otherwise I agree with you.


> mmmh, now, i see that you set found to 1 only if newprice
> <int(iprice)... new_price is a float (newprice = round(dprice)*1.10)
> that you compare with an int? is that correct? seems strangee to me.



There's nothing wrong with comparing floats to ints.

However, possibly iprice is not intended to be an int. The code is rather
confused and the names are at best obscure and at worst actively
misleading. I assumed that prices must be ints, (iprice means "integer
price"?) but that might not be the case, since later on another price is
multiplied by 1.10.

Also I wonder if the code is meant to calculate the new price:

newprice = round(dprice)*1.10 # dprice is the price in the CSV file!

or perhaps it is meant to be:

newprice = int(round(dprice*1.10))


That seems likely.


--
Steven

Rotwang 12-06-2012 03:22 AM

Re: Confused compare function :)
 
On 06/12/2012 00:19, Bruno Dupuis wrote:
> [...]
>
> Another advice: never ever
>
> except XXXError:
> pass
>
> at least log, or count, or warn, or anything, but don't pass.


Really? I've used that kind of thing several times in my code. For
example, there's a point where I have a list of strings and I want to
create a list of those ints that are represented in string form in my
list, so I do this:

listofints = []
for k in listofstrings:
try:
listofints.append(int(k))
except ValueError:
pass

Another example: I have a dialog box with an entry field where the user
can specify a colour by entering a string, and a preview box showing the
colour. I want the preview to automatically update when the user has
finished entering a valid colour string, so whenever the entry field is
modified I call this:

def preview(*args):
try:
previewbox.config(bg = str(entryfield.get()))
except tk.TclError:
pass

Is there a problem with either of the above? If so, what should I do
instead?


--
I have made a thing that superficially resembles music:

http://soundcloud.com/eroneity/we-be...-own-crapiness

Steven D'Aprano 12-06-2012 04:32 AM

Re: Confused compare function :)
 
On Thu, 06 Dec 2012 03:22:53 +0000, Rotwang wrote:

> On 06/12/2012 00:19, Bruno Dupuis wrote:
>> [...]
>>
>> Another advice: never ever
>>
>> except XXXError:
>> pass
>>
>> at least log, or count, or warn, or anything, but don't pass.

>
> Really? I've used that kind of thing several times in my code. For
> example, there's a point where I have a list of strings and I want to
> create a list of those ints that are represented in string form in my
> list, so I do this:
>
> listofints = []
> for k in listofstrings:
> try:
> listofints.append(int(k))
> except ValueError:
> pass
>
> Another example: I have a dialog box with an entry field where the user
> can specify a colour by entering a string, and a preview box showing the
> colour. I want the preview to automatically update when the user has
> finished entering a valid colour string, so whenever the entry field is
> modified I call this:
>
> def preview(*args):
> try:
> previewbox.config(bg = str(entryfield.get()))
> except tk.TclError:
> pass
>
> Is there a problem with either of the above? If so, what should I do
> instead?


They're fine.

Never, ever say that people should never, ever do something.


*cough*


--
Steven

Bruno Dupuis 12-06-2012 08:49 AM

Re: Confused compare function :)
 
On Thu, Dec 06, 2012 at 04:32:34AM +0000, Steven D'Aprano wrote:
> On Thu, 06 Dec 2012 03:22:53 +0000, Rotwang wrote:
>
> > On 06/12/2012 00:19, Bruno Dupuis wrote:
> >> [...]
> >>
> >> Another advice: never ever
> >>
> >> except XXXError:
> >> pass
> >>
> >> at least log, or count, or warn, or anything, but don't pass.

> >
> > Really? I've used that kind of thing several times in my code. For
> > example, there's a point where I have a list of strings and I want to
> > create a list of those ints that are represented in string form in my
> > list, so I do this:
> >
> > listofints = []
> > for k in listofstrings:
> > try:
> > listofints.append(int(k))
> > except ValueError:
> > pass
> >
> > Another example: I have a dialog box with an entry field where the user
> > can specify a colour by entering a string, and a preview box showing the
> > colour. I want the preview to automatically update when the user has
> > finished entering a valid colour string, so whenever the entry field is
> > modified I call this:
> >
> > def preview(*args):
> > try:
> > previewbox.config(bg = str(entryfield.get()))
> > except tk.TclError:
> > pass
> >
> > Is there a problem with either of the above? If so, what should I do
> > instead?

>
> They're fine.
>
> Never, ever say that people should never, ever do something.
>
>
> *cough*
>


Well, dependening on the context (who provides listofstrings?) I would
log or count errors on the first one... or not.

On the second one, I would split the expression, because (not sure of
that point, i didn't import tk for years) previewbox.config and
entryfield.get may raise a tk.TclError for different reasons.

The point is Exceptions are made for error handling, not for normal
workflow. I hate when i read that for example:

try:
do_stuff(mydict[k])
except KeyError:
pass

(loads of them in many libraries and frameworks)
instead of:

if k in mydict:
do_stuff(mydict[k])

Note that the performances are better with the latter.

There are some exceptions to this, though, like StopIteration

For me, it's a rule of thumb, except: pass is possible in situations
where I control every input data, and I deeply, exactly know all code
interractions. If figuring all this out is longer (it's almost always
the case) than typing:

log.warning('oops:\n %s' % traceback.format_exc())

I log.

It depends also on the context, I'd be more 'permissive' a short
script than into a large program, framework, or lib, for the
very reason it's easy to know all code interactions.

In my coder life, i spent more time debugging silently swallowed exceptions
than logging abnormal behaviours.

--
Bruno Dupuis

Chris Angelico 12-06-2012 09:34 AM

Re: Confused compare function :)
 
On Thu, Dec 6, 2012 at 7:49 PM, Bruno Dupuis
<python.ml.bruno.dupuis@lisael.org> wrote:
>
> The point is Exceptions are made for error handling, not for normal
> workflow. I hate when i read that for example:
>
> try:
> do_stuff(mydict[k])
> except KeyError:
> pass
>
> (loads of them in many libraries and frameworks)
> instead of:
>
> if k in mydict:
> do_stuff(mydict[k])
>
> Note that the performances are better with the latter.
>


This is the age-old question of EAFP vs LBYL.

The check-first "Look Before You Leap" option has a small chance of
race condition. If something changes between the 'if' and the usage,
maybe from another thread or maybe a signal handler or perhaps some
object's __del__ method gets called or who knows what, you'll have a
problem.

Python's usual philosophy is that it's Easier to Ask Forgiveness than
Permission. Just do it, and jump out if you can't. This technique
plays *very* nicely with generic handlers. For instance, at work I
wrote an HTTP daemon that's supposed to receive XML-encoded POST data
with a particular structure. The handler function (called once for
every HTTP request) looks something like this, in Pythonesque
pseudocode:

def handler(req):
try:
msg = parse_xml(req.body) # returns a dict of dicts/lists/strings
stuff = msg["soapenv:Envelope"]["soapenv:Body"]["GetItemTransactionsResponse"]
sig = stuff["Item"]["ApplicationData"]
if sig.does.not.match(): return "Bad request"
sscanf(sig,"FOO %d %d %d",account,table,row)
accountdata[account][table][row] = 1
return "Done and successful."
except:
log_error_to_stderr()
return "Done."

I don't particularly care _what_ the error is. Most of the time, I
won't even bother to look at the log file (it's run via an Upstart
job, and stderr is redirected to a file), but if I'm having problems,
I can go check. Generally, exceptions thrown by that code are the
result of malformed info packets; since it's basic HTTP, it's easy for
anyone to send a request in, and I don't care to see those errors
logged. In fact, the logging to stderr can even get commented out in
production, and used only when there's actually a problem being
diagnosed.

To try to handle all possible errors in that code by LBLY, I would
need to pepper the code with conditions and an appropriate 'return'
statement (and, though the pseudo-code has the function returning a
string, the actual code involves an explicit "send this response"
call). Plus, I'd need to predict every possible failure. With EAFP,
all I need is one simple "catch" handler for the whole block of code,
and I can easily eyeball just a few function exit points to see that
an appropriate HTTP response will always be sent.

Each has its place.

ChrisA

Steven D'Aprano 12-06-2012 11:47 AM

Re: Confused compare function :)
 
On Thu, 06 Dec 2012 09:49:26 +0100, Bruno Dupuis wrote:

> The point is Exceptions are made for error handling, not for normal
> workflow.


That's certainly not the case in Python. Using exceptions for flow
control is a standard part of the language.

IndexError and StopIteration are used to detect the end of lists or
iterators in for loops.

GeneratorExit is used to request that generators exit.

SysExit is used to exit the interpreter.


There is nothing wrong with using exceptions for flow control in
moderation.


> I hate when i read that for example:
>
> try:
> do_stuff(mydict[k])
> except KeyError:
> pass
>
> (loads of them in many libraries and frameworks) instead of:
>
> if k in mydict:
> do_stuff(mydict[k])
>
> Note that the performances are better with the latter.


Not so. Which one is faster will depend on how often you expect to fail.
If the keys are nearly always present, then:

try:
do_stuff(mydict[k])
except KeyError:
pass

will be faster. Setting up a try block is very fast, about as fast as
"pass", and faster than "if k in mydict".

But if the key is often missing, then catching the exception will be
slow, and the "if k in mydict" version may be faster. It depends on how
often the key is missing.


[...]
> It depends also on the context, I'd be more 'permissive' a short script
> than into a large program, framework, or lib, for the very reason it's
> easy to know all code interactions.
>
> In my coder life, i spent more time debugging silently swallowed
> exceptions than logging abnormal behaviours.



That's fine. I agree with you about not silently swallowing errors. Where
I disagree is that you said "never ever", which is an exaggeration.
Remember that exceptions are not always errors.

Problem: take a list of strings, and add up the ones which are integers,
ignoring everything else.

Solution:

total = 0
for s in list_of_strings:
try:
total += int(s)
except ValueError:
pass # Not a number, ignore it.


Why would you want to log that? It's not an error, it is working as
designed. I hate software that logs every little thing that happens, so
that you cannot tell what's important and what isn't.



--
Steven

peter 12-06-2012 11:55 AM

Re: Confused compare function :)
 
On 12/06/2012 08:47 AM, Steven D'Aprano wrote:
> On Thu, 06 Dec 2012 09:49:26 +0100, Bruno Dupuis wrote:
>
>> The point is Exceptions are made for error handling, not for normal
>> workflow.

> That's certainly not the case in Python. Using exceptions for flow
> control is a standard part of the language.
>
> IndexError and StopIteration are used to detect the end of lists or
> iterators in for loops.
>
> GeneratorExit is used to request that generators exit.
>
> SysExit is used to exit the interpreter.
>
>
> There is nothing wrong with using exceptions for flow control in
> moderation.
>
>
>> I hate when i read that for example:
>>
>> try:
>> do_stuff(mydict[k])
>> except KeyError:
>> pass
>>
>> (loads of them in many libraries and frameworks) instead of:
>>
>> if k in mydict:
>> do_stuff(mydict[k])
>>
>> Note that the performances are better with the latter.

> Not so. Which one is faster will depend on how often you expect to fail.
> If the keys are nearly always present, then:
>
> try:
> do_stuff(mydict[k])
> except KeyError:
> pass
>
> will be faster. Setting up a try block is very fast, about as fast as
> "pass", and faster than "if k in mydict".
>
> But if the key is often missing, then catching the exception will be
> slow, and the "if k in mydict" version may be faster. It depends on how
> often the key is missing.
>
>
> [...]
>> It depends also on the context, I'd be more 'permissive' a short script
>> than into a large program, framework, or lib, for the very reason it's
>> easy to know all code interactions.
>>
>> In my coder life, i spent more time debugging silently swallowed
>> exceptions than logging abnormal behaviours.

>
> That's fine. I agree with you about not silently swallowing errors. Where
> I disagree is that you said "never ever", which is an exaggeration.
> Remember that exceptions are not always errors.
>
> Problem: take a list of strings, and add up the ones which are integers,
> ignoring everything else.
>
> Solution:
>
> total = 0
> for s in list_of_strings:
> try:
> total += int(s)
> except ValueError:
> pass # Not a number, ignore it.
>
>
> Why would you want to log that? It's not an error, it is working as
> designed. I hate software that logs every little thing that happens, so
> that you cannot tell what's important and what isn't.
>
>
>

Is perfectly right to use try catch for a flow control.
Just think in something more complex like this.

try:
self._conn = MySQLdb.connect(host=host,
user=user,
passwd=passwd,
db=db)
except:
logging.info("Error de conexion con la base de datos")
inform(subject = 'Db down on app %s' % app, body=sbody)

Or maybe something like this.

try:
cursor.execute(sqli, data)
self._conn.commit()
except:
try:
self._conn.rollback()
cursor.execute(sqli, data)
self._conn.commit()
except Exception, e:
pass
# print e
# logging.info('ERROR en la insercion %s' % e)

This is pretty dumb, but is a valid example, on what you can do with try
catch

Chris Angelico 12-06-2012 12:14 PM

Re: Confused compare function :)
 
On Thu, Dec 6, 2012 at 10:47 PM, Steven D'Aprano
<steve+comp.lang.python@pearwood.info> wrote:
> Not so. Which one is faster will depend on how often you expect to fail.
> If the keys are nearly always present, then:
>
> try:
> do_stuff(mydict[k])
> except KeyError:
> pass
>
> will be faster. Setting up a try block is very fast, about as fast as
> "pass", and faster than "if k in mydict".
>
> But if the key is often missing, then catching the exception will be
> slow, and the "if k in mydict" version may be faster. It depends on how
> often the key is missing.
>


Setting up the try/except is a constant time cost, while the
duplicated search for k inside the dictionary might depend on various
other factors. In the specific case of a Python dictionary, the
membership check is fairly cheap (assuming you're not the subject of a
hash collision attack - Py3.3 makes that a safe assumption), but if
you were about to execute a program and wanted to first find out if it
existed, that extra check could be ridiculously expensive, eg if the
path takes you on a network drive - or, worse, on multiple network
drives, which I have had occasion to do!

ChrisA

Hans Mulder 12-06-2012 01:32 PM

Re: Confused compare function :)
 
On 6/12/12 12:55:16, peter wrote:
> Is perfectly right to use try catch for a flow control.
> Just think in something more complex like this.
>
> try:
> self._conn = MySQLdb.connect(host=host,
> user=user,
> passwd=passwd,
> db=db)
> except:
> logging.info("Error de conexion con la base de datos")
> inform(subject = 'Db down on app %s' % app, body=sbody)


This is an example of the sort of incorrect code you
should try to avoid. An improved version is:

self._conn = MySQLdb.connect(host=host,
user=user,
passwd=passwd,
db=db)

By not catching the exception, you're allowing the
Python interpreter to report what the problem was,
for example "Keyboard interrupt" or "Access denied".

By report "DB down" when there is no reason to assume
that that is the problem, you're confusing the user.

> Or maybe something like this.
>
> try:
> cursor.execute(sqli, data)
> self._conn.commit()
> except:
> try:
> self._conn.rollback()
> cursor.execute(sqli, data)
> self._conn.commit()
> except Exception, e:
> pass
> # print e
> # logging.info('ERROR en la insercion %s' % e)


This is another example of what not to do. Even the
commented-out print statement loses information, viz.
the traceback.

If you leave out the try/except, then more accurate
information will be printed, and a programmer who needs
to fix the problem, can run the code under the debugger
and it will automatically stop at the point where the
uncaught exception is raised. That's much easier than
having to set breakpoints at all the "except Exception:"
clauses in a typical chunk of hard-to-maintain code.

Context managers were invented to make it easier to do
this sort of thing correctly. For example:

with sqlite3.connect(dbpath) as connection:
connection.cursor().execute(sqli, data)

If the flow reaches the end of the "with" command,
the connection object will self.commit() automatically.
If an exception is raised, the connection object will
self.rollback() automatically. No try/except required.

This is shorter, and much easier to get right.

> This is pretty dumb, but is a valid example, on what you can
> do with try catch


It is an unfortunate fact of life that you can write code
that is hard to maintain. The fact that you *can* do this,
does not mean that you should.


Hope this helps,

-- HansM



All times are GMT. The time now is 01:06 PM.

Powered by vBulletin®. Copyright ©2000 - 2014, vBulletin Solutions, Inc.
SEO by vBSEO ©2010, Crawlability, Inc.