Normalize database config file vs cursor etc #184
Conversation
…use cursors and remove config file pollution.
| extraction = None | ||
| if self.cursor is not None: | ||
| extraction = self._extract(specimen=specimen, study=study, continuous_also=continuous_also) | ||
| if self.database_config_file is not None: |
There was a problem hiding this comment.
Shouldn't this be an elif? (Although it wouldn't be an issue if only one of cursor or database_config_file were saved in the init, but even then an elif should probably be used for better clarity.)
There was a problem hiding this comment.
Yes this is a mistake. The IDE suggested changing elif to if and I complied, but that was when those extract lines were return statements. In this form elif is right.
| @@ -1,10 +1,10 @@ | |||
| """ | |||
| Convenience provision of a feature matrix for each study, the data retrieved | |||
| from the SPT database. | |||
| Convenience provision of a feature matrix for each study, the data retrieved from the SPT database. | |||
| """ | |||
There was a problem hiding this comment.
pydocstyle says module docstrings should be one liners if it fits on one line (include the start and end quotes). If not, there should be a short description that fits on one line and elaboration after two line breaks.
There was a problem hiding this comment.
Hm yes I've thought about this sometimes. There is an 8 character ambiguity, since making a one-line comment block into one comment line increases the size of the line by the size of """ """. In this case, this goes beyond the 100 character line limit I've been using.
It seems that the pydocstyle prescription asks to alter the content of the docstring in this case. This seems like arbitrary carelessness on the part of the style guide, since it follows that docstrings that are between 93 and 100 characters long are not allowed.
There was a problem hiding this comment.
I acquiesce and reword to make it more concise, putting further detail in the body if it calls for it. Your effective short description limit is about 10 characters shorter than your chosen line limit setting.
spatialprofilingtoolbox/workflow/common/sparse_matrix_puller.py
Outdated
Show resolved
Hide resolved
spatialprofilingtoolbox/workflow/common/sparse_matrix_puller.py
Outdated
Show resolved
Hide resolved
|
I think everything is fixed up. |
|
Final change approved. |
Addresses #182 .