Hi Luca, On Mon, 24 Jan 2022 at 13:50, Luca Weiss <luca@xxxxxxxxx> wrote: > > Hi Simon, > > On Montag, 24. Jänner 2022 18:57:45 CET Simon Glass wrote: > > Hi Luca, > > > > On Sat, 22 Jan 2022 at 03:36, Luca Weiss <luca@xxxxxxxxx> wrote: > > > Hi Simon, > > > > > > On Dienstag, 28. Dezember 2021 09:34:57 CET Simon Glass wrote: > > > > Hi Luca, > > > > > > > > On Sat, 25 Dec 2021 at 06:26, Luca Weiss <luca@xxxxxxxxx> wrote: > > > > > Add a new method that doesn't throw an exception when a property isn't > > > > > found but returns None instead. > > > > > > > > > > Also add a test for the new method. > > > > > > > > You can use > > > > > > > > getprop(node, prop, quiet=QUIET_NOTFOUND) > > > > > > This returns -1 when not found which I found quite un-pythonic (not that > > > pylibfdt is currently super pythonic ;) ). > > > Having None returned if not found is much nicer to use and much more clear > > > to use > > > > > > if foobar is None: > > > instead of having to use > > > > > > if foobar == -QUIET_NOTFOUND: > > > Are there any changes I can do for you to reconsider your position on this > > > patch? > > > > I think I was put off by the mention of an exception in the commit > > message. Really all you are doing is trying to make this function more > > pythonic. I suppose we could change the getprop() function to return > > None if there is no property (with QUIET_NOTFOUND supplied for the > > 'quiet' argument). That makes not-found special, but I suppose if the > > error were anything else, e.g. -FDT_ERR_BADSTRUCTURE and > > quiet=[BADSTRUCTURE] were passed, we could return None in that case > > too? > > > > I agree it is more pythonic. > > The "proper" way (in my opinion) would be to return the property or None, > except for more serious errors like the BADSTRUCTURE one that just throws an > exception; and hide all the C integer return values inside the wrapper. > And instead of passing quiet=[BADSTRUCTURE] you would do You could return None for the ones that are quietened, but then note that you cannot recover the actual error. So in fact I think that is a problem with what I proposed above, so in fact we should only return None for -FDT_ERR_NOTFOUND, if even that. > > try: > myprop = fdt.getprop(foobar) > except FdtBadStructureError: > foobar() > > I understand it's probably not a good idea to change the existing function too > much given current users where scripts depend on this behavior but this way > it's definitely more approachable to an average Python user. Unless you are possibly expecting an error, e.g. -FDT_ERR_NOSPACE. It is a pain to have to catch exceptions for what is normal operation. In fact if I recall the Python sages recommend against things like that? Regards, Simon > > > > David, what do you think? > > > > Failing that I'm OK with this function, but with the commit message > > updated to not mention exceptions, as we already have that mechanism, > > so it is confusing. > > > > BTW you can use QUIET_NOTFOUND instead of [FDT_ERR_NOTFOUND] > > > > Regards, > > Simon > > > >
