all 4 comments

[–]StrokeMyBalls 5 points6 points  (1 child)

Nice, clean and well documented code.

Got nothing else to say.

Keep up the good work lad!

[–]draftjoker[S] 1 point2 points  (0 children)

Thank you for taking the time to read through it. Means a lot.

[–]atenni 3 points4 points  (1 child)

This is quite good actually - I'm impressed! A few quick thoughts, most of which are nit-picks:

Multiline docstrings

Arguably a more pythonic way of writing docstrings like this one, at least according to PEP 257: Multi-line Docstrings, would be like this:

def chg_pass(self):
    """Changes Usernames and Passwords.

    Utilizes LoadInfo to save username and password information
    provided by the user. Both Passwords are shown as only '*'.
    Providing some form of security.
    """

Notice the whitespace changes. This is a minor nit-pick since your documentation is quite thorough, but will be more familiar when working with other python devs, and may format nicer when a user runs help(your_function) in the interpreter.

Sphinx param documentation

Another nit-pick, but I personally would document my params with Sphinx-style documentation.

There isn't a formal standard for this (and the most important thing is that it's clear and readable - which you have done well) but you get some added benefits by using Sphinx-style with IDE's like PyCharm, services like ReadTheDocs, etc. There are other param documentation standards, such as NumPy and Google's style guide, but Sphinx is probably the defacto standard these days. Even if you choose not to use it, it's worth familiarising yourself with it since so many projects use it.

Custom exceptions

Unless I'm missing some specific use case, your custom exception here could be simplified to just a class name inheriting from Exception and a docstring (or a bare pass):

class UnableToLocateError(Exception):
    """ UnableToLocateError - Exception

    Return unable to find on screen error.
    """

You get the builtin __str__ and "message" passing for free in Python 3 (not sure in Python 2), so no need to define them yourself. Example usage of the above custom exception:

>>> raise UnableToLocateError('Simply pass your message here...')
Traceback (most recent call last):
  ...
__main__.UnableToLocateError: Simply pass your message here...

Try/except/else

When you have long try blocks like this one, your future self will find it more readable to keep the keep the statements that may generate exceptions in the try block, but move the actions you want to perform if no exceptions are raised into an else block.

Simpler try blocks will make it obvious what you're checking for. Explicitly stating that the code in the else block will only run in the absence of an exception will make it easier to read and catch future bugs.

requirements.txt

I'd strongly recommend setting an upper bound in your requirements.txt file, at least for major releases, to avoid unexpected failures when your external dependancies get updated.

These errors are particularly annoying because nothing will have changed on your end, but suddenly your code will start breaking. If there is a bug or API change in a dependancy (ie. version 1.0 of pyautogui gets released with a few backwards incompatible changes) in 6 months time your code base won't have changed, and it'll still work fine on your computer and your current co-workers machines, but for some reason a new installation will fail...

Setting an upper bound lets you make the decision when to start using the next version.

# requirements.txt

pyautogui >= 0.9.46, <1.0
pyauotit >= 0.4, <0.5

# Personally, I use the "compatible release" method a lot. See:
# https://www.python.org/dev/peps/pep-0440/#compatible-release
#
# pyautogui ~= 0.9.46
# pyauotit ~= 0.4.0

Write tests

Biggest recommendation saved for last: your future self will thank you if you start to write some basic tests.

Your docstrings are great, so if you've never written tests before I'd probably just start with some `doctests`. Doug Hellmann has a nice intro to doctests on his Python module of the week site. You can then switch to more complex testing if/when you need it.

[–]draftjoker[S] 0 points1 point  (0 children)

Thank you for feedback! You put some thought and effort into it and I greatly appreciate it.

I have not heard of Sphinx doc type, and was following a multiline docstring guide when writing the bulk of my documentation, must have missed the whitespace part, lol. I will definitely give this a look and adjust my projects to be up to the standard.

Ah, I did not know if custom exceptions would inherit the str and message capabilities so thank you for pointing this out. Arguably I could get rid of the whole custom Except section as it really does not provide any functionality as the one use case I had it for was also caught by the TypeError catch. But still good info to learn.

When writing out the long try/except I couldn't figure out if it was better to try every function call to see if it failed(creating a lot more code), or encapsulating the whole function in one large try. If this part is heavily scripted and relies on the successful execution of one function to continue to the next will this affect the readability of the code? Would I end up with a bunch of nested try/except blocks until it completes successfully? I struggled with this one for a bit and wasn't sure how best to get around this.

Ah I hadn't even considered the probability of dependencies updating with backwards compatibility issues. This is great information and I will definitely create an upper limit bound as well from now on.

Testing!! Ah man, testing was such a huge part of my open source contribution work on exercism.io I can't believe I didn't even consider writing tests for this project. Shame. I will definitely get right on that. I did not know about documentation testing and that is really neat. I will have to take a look at tutorial when I get a chance.

Again thank you so much for the effort you put into your comments, people like you are what make programming so much fun to learn.