Closed Slowblitz closed 3 years ago
Hello @Slowblitz! Thanks for updating this PR.
ando/AnDOChecker.py
:Line 133:15: W291 trailing whitespace
ando/tools/generator/AnDOGenerator.py
:Line 30:101: E501 line too long (106 > 100 characters) Line 109:101: E501 line too long (113 > 100 characters) Line 261:1: W293 blank line contains whitespace Line 280:1: W293 blank line contains whitespace
This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Changes Missing Coverage | Covered Lines | Changed/Added Lines | % | ||
---|---|---|---|---|---|
ando/tools/generator/tests/test_AnDOGenerator.py | 0 | 1 | 0.0% | ||
ando/tools/generator/AnDOGenerator.py | 0 | 38 | 0.0% | ||
<!-- | Total: | 6 | 45 | 13.33% | --> |
Files with Coverage Reduction | New Missed Lines | % | ||
---|---|---|---|---|
ando/AnDOChecker.py | 10 | 71.79% | ||
ando/tools/generator/AnDOGenerator.py | 42 | 0.0% | ||
<!-- | Total: | 52 | --> |
Totals | |
---|---|
Change from base Build 722690554: | 0.0% |
Covered Lines: | 203 |
Relevant Lines: | 494 |
Thanks for the update and sorry for not changing the doc style already when porting the generator code... Do you have a readthedocs test project somewhere so we can already see the result of these changes online?
@JuliaSprenger Yes I've updated in the main description at the top but here it is : https://ando-test.readthedocs.io/en/enh-doc_standardisation/
Can you please also substitute the bit.ly link by the 'official' BIDS/BEP032 link?
Hi @Slowblitz!
Here a first list of things I noticed when scanning the documentation. We should also thing about harmonizing variable names at some point, e.g. path
, dirpath
, dir
...
ando.AnDOChecker.search
method has no docstring yet.ando.AnDOChecker.is_valid
needs a more specific return description: it returns a tuple of size 2, the first entry being the bool value, the 2nd a listando.AnDOChecker.main()
has inconsistent names in the example: path
and directory
is the same variableando.AnDOChecker.main()
optional arguments should be ideally represented as a list and not one long stringando.AnDOChecker.is_valid
docstring needs to be replaced by to the BIDS/BEP032 link AnDOData.validate
has mixed up documentation of returns:
and return type
.AnDOData.register_data_files
docstring is missing a space between files
and are
AnDOData.register_metadata_files
needs a docstringAnDOData
Parameter example for ses_id
is invalid as it contains -
"The google doc link in the ando.AnDOChecker.is_valid docstring needs to be replaced by to the BIDS/BEP032 link " the link is already the BEP's one for me or is there an other link ?
here is the link to be used (cf our discussion on mattermost): https://bids.neuroimaging.io/bep032
Thanks @SylvainTakerkart !
register_metadata
method? Something like
` """
Register metadata with the AnDO data structure.
Parameters *files : path to files to be added as metadata files.
File content needs to be according with AnDO guidelines as files will only be moved to the their correct location based on the file name
"""
And I guess this PR is not a draft any more, but ready for review?
I started going through the documentation on readthedocs again and found some more issues:
Can be either ‘link’ ‘copy’ or ‘move’.
register_metadata_files()
method has broken layout of the docstring - parameters are not recognized as parametersgenerate_struct
for PEP8 compatiblity. Docstring is missing a dot at the end of the first sentence. ESSENTIAL_CSV_COLUMNS = [‘sub_id’, ‘ses_id’]
should read . Essential columns are 'sub_id' and 'ses_id'.
Done =)
Here is a draft to make sure all docstrings in our project uses the same convention, we have decided to use the Numpy Style Python Docstrings. Example here: https://sphinxcontrib-napoleon.readthedocs.io/en/latest/example_numpy.html#example-numpy Here's the result before merging it on the main repos : https://ando-test.readthedocs.io/en/enh-doc_standardisation/ Feel free to add documentation where it's needed. Happy coding.