chore: update pre-commit#199
Conversation
|
@stevenhua0320, ready to review |
|
@danielsirakov Have you run the |
|
I don't love the changes that docformatter is introducing here either, by capitalizing the type. This could be our fault. It is probably not a good idea to use the |
|
@sbillinge I have looked at the docstring changes that changed by to For some of the attribute definition string(the latter case), what I suggest is shadow the type of attribute with a pair of quote. So we ca do for example to |
sbillinge
left a comment
sbillinge
left a comment
There was a problem hiding this comment.
I gave some thought here. Please can we try and make these fixes when finals are over and move this to a p3.14 release?
|
|
||
| BtoU = 1.0 / (8 * numpy.pi**2) | ||
| """float: Conversion factor from B values to U values.""" | ||
| """Float: Conversion factor from B values to U values.""" |
There was a problem hiding this comment.
@danielsirakov @stevenhua0320 we don't want string literals floating around in the code, so whenever we find things like this we want to clean them up. The correct fix in general is to move them to the docstring at the top of the function and put them into a standard format. Here, this may just be a constant and this should be changed from a string to a comment and placed above line 154.
| "END", | ||
| ] | ||
| """list: Ordered list of PDB record labels.""" | ||
| """List: Ordered list of PDB record labels.""" |
There was a problem hiding this comment.
another floating string literal. It is documenting the static data list. Gemini suggested that even though it is not declared as self. it is still considered as a class attribute and so should be documented in the class docstring, something like:
class P_pdb(StructureParser):
"""Simple parser for PDB format.
The parser understands following PDB records: `TITLE, CRYST1, SCALE1,
SCALE2, SCALE3, ATOM, SIGATM, ANISOU, SIGUIJ, TER, HETATM, END`.
Attributes
----------
format : str
Format name, default "pdb".
orderOfRecords : list
The ordered list of PDB record labels.
validRecords : dict
The set of valid PDB record labels.
"""
We can discuss this more, but in general I think taking this advice and using it also in other places in the PR is the right approach
| # instance attributes that have immutable default values | ||
| element = "" | ||
| """str: Default values of `element`.""" | ||
| """Str: Default values of `element`.""" |
There was a problem hiding this comment.
this code seems just to be setting defaults, so the default values just have to be moved up to the attribute description in the docstring above and deleted from here, something like that.
| # default values for instance attributes | ||
| title = "" | ||
| """str: default values for `title`.""" | ||
| """Str: default values for `title`.""" |
There was a problem hiding this comment.
this is default setting again.
3 participants
I re-did this as I forgot the news file