Skip to content
Snippets Groups Projects

Get set generic

Merged Karel Koci requested to merge get_set_generic into master
2 unresolved threads

This creates all powerful get and set methods. The idea behind this is to protect input from configuration. Uci does not provide us with types so if we want to be sure that we are working with correct type (which can be pretty funny like iteration over tuple vs. string) we have to check that. These new all mighty and ultimate methods are intended to do so.

This integrates all previously defined functions to single one.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • added Feature label

  • Author Contributor

    @shenek cam you please also go trough this new API and make sure that it does not break anything for you? Or potentially if you see something obviously wrong..

  • Karel Koci added 2 commits

    added 2 commits

    • c1ddf063 - euci: add support for getting and settings lists
    • 4a110faa - euci: add option to ensure that tupple is returned

    Compare with previous version

  • Karel Koci added 1 commit

    added 1 commit

    • d9ec725f - euci: improve exception when invalid keyword argument is passed

    Compare with previous version

  • Martin Matějek
    Martin Matějek @mmatejek started a thread on the diff
    Last updated by Karel Koci
    126 according to that. Supported types are: str, bool and int.
    127 If provided value is not of any of these types then it is converted to
    128 string.
    129
    130 It is suggested to always explicitly type last positional argument to
    131 ensure correct type. That is in case of boolean for example:
    132 set("foo", "fee", "faa", bool(value))
    63 133 """
    64 value = self.get(*args, **kwargs)
    65 if value.lower() not in self.__BOOLEAN_VALUES:
    66 raise ValueError
    67 return self.__BOOLEAN_VALUES[value.lower()]
    134 dtype = type(args[-1])
    135 if dtype == tuple or dtype == list:
    136 # We consider first value as authoritative for type
    137 dtype = type(args[-1][0]) if args[-1] else str
    • Can you elaborate a bit?

      args[-1] will be always present so check for args[-1] wouldn't help if there is empty list or tuple and type(args[-1][0]) should fail on that. Or is it working due to some sort of special behaviour of ternary operator comparison?

    • Author Contributor

      That args[-1] is expanded to tuple/list that is accessed in args[-1][0] so it is effectively doing bool(args[-1]) which means: is tuple/list empty?
      If it is empty then we fallback on string type. It is effectively not used later but I did it for consistency sake.

      And yes we expect that args[-1] is always present.

    • Please register or sign in to reply
  • Martin Matějek
    Martin Matějek @mmatejek started a thread on the diff
    Last updated by Karel Koci
    57 99
    58 def get_boolean(self, *args, **kwargs):
    59 """Returns given UCI config as a boolean.
    60 Value '0', 'no', 'off', 'false' or 'disabled' is returned as False.
    61 Value '1' , 'yes', 'on', 'true' or 'enabled' is returned as True.
    62 ValueError is raised on any other value.
    100 if type(values) == tuple:
    101 result = tuple((self._get(str(value), dtype) for value in values))
    102 else:
    103 result = self._get(str(values), dtype)
    104 if 'list' in kwargs:
    105 if (type(result) == tuple) == bool(kwargs['list']):
    106 return result
    107 if kwargs['list']:
    108 return (result,)
    109 return result[0]
    • Just to be sure. Isn't there possibility that when you want to get single option and set list=False then you'll get first character of string or exception in case it isn't dtype str?

    • Author Contributor

      In case of list=False and type(result) != tuple then (type(result) == tuple) == bool(kwargs['list']) evaluates to False == False which is True and so result is returned as it is.

    • All right, I was wondering why there is that (type(result) == tuple) == bool(kwargs['list']) comparison. It is a little bit cryptic and hard to read at first glance, but apart from that it looks functional.

    • Author Contributor

      It is complicated condition I agree but it is I think better than duplicating code.

      Edited by Karel Koci
    • Please register or sign in to reply
  • It looks good otherwise.

  • assigned to @kkoci

  • Karel Koci added 1 commit

    added 1 commit

    • 064a057e - euci: accept any iterable in set method

    Compare with previous version

  • assigned to @mmatejek

  • Author Contributor

    I just added one more commit which improves code. It should have been implemented that way from beginning but it wasn't so please review that small change. Thank you.

  • This small change and code overall looks good. 👍

  • assigned to @kkoci

  • merged

Please register or sign in to reply