Fixing prepare step bug

[rob-c]

This cleans up some code in the IPrepareApp and gives a hook to allow derrived classes to quickly and easily copy a file into the prepared state folder during the prepare step.
This is useful for my LSST work where I'm already using this and it fixes a typo which was left from this function already existing 4x in the exiting code.

[milliams]

Makes sense to me. It's not obvious from the docstring what type obj2copy should be. Adding argument descriptions to the docstring in the Google style would be useful as these are also parsed by Sphinx when generating the documentation.

Also, we could potentially consider starting to use PEP 484-style comments to define types.

[rob-c]

@milliams I'll glance at the links later but I just wanted to make sure I don't end up with a huge commit addressing lots of bugs again in the future.

[rob-c]

OK, frankly this is why I prefer strong typed languages. I've no idea what to do with the documentation I'm simply going to add a line stating that this is expected to be a string due to the shutil.copy2 function

[milliams]

For the record the Google-style docstring looks like:

def copyIntoPrepDir(self, obj2copy):
    """
    Copy a file into the prepared state dir of this application

    Args:
        obj2copy (str): the path to the file to be copied
    """

And if you wanted to use PEP 484 comment, then it would look like:

def copyIntoPrepDir(self, obj2copy):
    # type: (str) -> None
    """
    Copy a file into the prepared state dir of this application

    Args:
        obj2copy (str): the path to the file to be copied
    """

[rob-c]

@milliams so what are we looking to standardize on?

[milliams]

For now I think just the docstring format is needed, so the first of the two. The latter is only useful if we start running static analysis tools like MyPy over Ganga or for those of us using PyCharm but until then they are arguably just noise.

@alexanderrichards has been arguing for the former for a while now too.

[alexanderrichards]

@alexanderrichards has been arguing for the former for a while now too.

👍 Indeed I have 😄.
Out of curiosity @milliams do you prefer the type comments from pep484 like in your second example or inline in the function prototype using the annotation style?

e.g.

def copyIntoPrepDir(self, obj2copy: str) -> None:
    """
    Copy a file into the prepared state dir of this application

    Args:
        obj2copy (str): the path to the file to be copied
    """

[rob-c]

I'm not going to start reading documentation on documentation, it's too meta and full of a lot of confusion when I'm trying to get other work done.
If I'm to write documentation like the google-style then OK, as long as it's clear what is expected. If there's a cheat sheet for those who don't care about how this works or what this does even better.

I'm sticking with the first example unless someone tells me otherwise. I want to fix and get this in so I can merge it into another branch where I want this functionality,

[alexanderrichards]

I'm sticking with the first example unless someone tells me otherwise.

I think we agree that the first example (The Google-style) one is the one we should aim for at the moment as we don't run any static analysis tools as I think that it is perfectly sufficient even when we do. The main reason I wanted us to do it this way is because it gives us the ability to auto-generate doc whilst maintaining the user friendly interactive help that users expect.

[milliams]

@alexanderrichards I much prefer the inline style but that is restricted to Python 3 only. The comment style was added to the PEP in the last few months to be able to provide the feature for Python 2 and to provide a forwards compatibility path.

@rob-c Doing documentation properly is part of the job I'm afraid. I'm not saying that you have to have an opinion on what is the best way but is is important in a collaborative project to help out your colleagues by documenting your work appropriately.

[alexanderrichards]

I much prefer the inline style but that is restricted to Python 3 only.

Ah yes forgot that 😉

[rob-c]

@milliams Doing documentation properly is part of the job I'm afraid. I am very aware of this and am extremely happy to document things as long as it's clear what I am doing and it is set out in a simple way what is expected of me.
This style formatting (as far as I'm aware) hasn't been discussed formally until today which is why I'm asking for a decision to be made by someone.
(I've documented the file as I prefer to do this all rather than doing just 1 function at a time as I've mentioned in the past.)

[milliams]

This comment does not add anything. In fact, this __init__ method should just be removed entirely.

[rob-c]

OK, once the final round of tests pass I plan to merge this unless there are any objections

[milliams]

Type on 'Fforces'

[rob-c]

This is going to take another 20min for the sake of a typo. I'll merge the PR and edit it on develop