Feedback on my first Python module

Feedback on my first Python module - Printable Version

+- Python Forum (https://python-forum.io)
+-- Forum: General (https://python-forum.io/forum-1.html)
+--- Forum: Code sharing (https://python-forum.io/forum-5.html)
+--- Thread: Feedback on my first Python module (/thread-10985.html)

Pages: 1 2

Feedback on my first Python module - CodeRaker - Jun-16-2018

Hi there Pyople Shy

,

I'm developing my first python module intended to assist with writing (for now) Ubuntu server administration scripts.
I'm not trying to invent the new Ansible - rather just learning to write a module and help shorten my administration scripts, by creating a generic module.

The code is here:
https://github.com/CodeRaker/automation

The module works. However I'm wondering if the module structure, the way I built the functions and so on is aligned with the Python way. I'ld like some pointers to improve/optimize the code. If anyone would be so kind to provide some feedback.

Sincerily,

Peter (CodeRaker)

RE: Feedback on my first Python module - Larz60+ - Jun-16-2018

There is a trick you can use to change any value to True or false:

λ python
Python 3.6.5 (v3.6.5:f59c0932b4, Mar 28 2018, 17:00:18) [MSC v.1900 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> x = 'Hello'
>>> not not x
True
>>> x = 0
>>> not not x
False
>>>

so when you write a statement like:

if result == 0:
    return True
else:
    return False

it can be shortened to:

return not not result

which will return True or False

RE: Feedback on my first Python module - ichabod801 - Jun-16-2018

Why not just use:

return bool(result)

RE: Feedback on my first Python module - CodeRaker - Jun-16-2018

(Jun-16-2018, 12:55 PM)Larz60+ Wrote: There is a trick you can use to change any value to True or false:
 λ python Python 3.6.5 (v3.6.5:f59c0932b4, Mar 28 2018, 17:00:18) [MSC v.1900 64 bit (AMD64)] on win32 Type "help", "copyright", "credits" or "license" for more information. >>> x = 'Hello' >>> not not x True >>> x = 0 >>> not not x False >>> 
so when you write a statement like:
 if result == 0: return True else: return False 
it can be shortened to:
 return not not result 
which will return True or False

Pretty cool. I had to write

return not result

in order for it to write True on 0 and False on any other number. Thanks for the contribution! Updated the module :)

RE: Feedback on my first Python module - buran - Jun-16-2018

@Larz - only one not will do
@ichabood - you need not

:-)

def larz(result):
    return not not result
    
def code_raker(result):
    if result == 0:
        return True
    else:
        return False

def ichabood(result):
    return bool(result)


print('x==0')
x=0
print('CodeRaker: {}'.format(code_raker(x)))
print('Larz: {}'.format(larz(x)))
print('Ichabood: {}'.format(ichabood(x)))

print('\nx==1')
x=1
print('CodeRaker: {}'.format(code_raker(x)))
print('Larz: {}'.format(larz(x)))
print('Ichabood: {}'.format(ichabood(x)))

Output:x==0
CodeRaker: True
Larz: False
Ichabood: False

x==1
CodeRaker: False
Larz: True
Ichabood: True

RE: Feedback on my first Python module - CodeRaker - Jun-16-2018

(Jun-16-2018, 02:36 PM)ichabod801 Wrote: Why not just use:
return bool(result)
?

In order for it to work I would have to write

return not bool(result)

So I chose the shorter version by Lars60+. Both ways much more clever than my initial approach. Thanks :)

RE: Feedback on my first Python module - buran - Jun-16-2018

some random thoughts:

1. do not do if verbose == True:. verbose is already True or False, so just do if verbose:
2. All this

if action == 'restart':
    run_command('systemctl restart ' + service, systemctl_verbose)
    if verbose == True:
        print('[V] Restarting service: ' + service)
if action == 'start':
    run_command('systemctl start ' + service, systemctl_verbose)
    if verbose == True:
        print('[V] Starting service: ' + service)
if action == 'stop':
    run_command('systemctl stop ' + service, systemctl_verbose)
    if verbose == True:
        print('[V] Stopping service: ' + service)
if action == 'reload':
    run_command('systemctl reload ' + service, systemctl_verbose)
    if verbose == True:
        print('[V] Reloading service: ' + service)

could be just

run_command('systemctl {} {}'.format(action, service), systemctl_verbose)
if verbose:
    print('[V] {} service: {}'.format(action.capitalize(), service))

3. this

with open(path) as infile:
    with open(path + '.new', 'w') as outfile:

can be on one line, to reduce indentation levels

with open(path) as infile, open(path + '.new', 'w') as outfile:

RE: Feedback on my first Python module - CodeRaker - Jun-16-2018

(Jun-16-2018, 03:43 PM)buran Wrote: could be just

run_command('systemctl {} {}'.format(action, service), systemctl_verbose) if verbose: print('[V] {} service: {}'.format(action.capitalize() + service))

Wow! Thanks for that. Whipped by the pythonic slapstick! Just had to change + to , in

print('[V] {} service: {}'.format(action.capitalize() + service))

then it works perfectly. Module has been updated. Awesome :)

RE: Feedback on my first Python module - buran - Jun-16-2018

(Jun-16-2018, 04:14 PM)CodeRaker Wrote: Just had to change + to , in

yes, sorry for the mistake. I updated it shortly after the post

RE: Feedback on my first Python module - snippsat - Jun-16-2018

Some use of string formatting but could be more,not so nice + ' ' +.
f-string makes it a lot nicer,that will of course make it 3.6 only.
if that's no problem then use f-string.

# You
run_command('mv ' + path + ' ' + path + '.old')

# f-string
run_command(f'mv {path} {path}.old')

# format() <-- 3.5
run_command('mv {0} {1}.old'.format(path, path))

with open() can be written in one statement,some like this as some not.

with open(path) as infile,open(f'{path}.new', 'w') as outfile:
    ...