-
-
Notifications
You must be signed in to change notification settings - Fork 404
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make docstrings consistent with numpydoc format #701
Conversation
In many places, docstrings violate the numpydoc format, hence breaking the type hinting in PyCharm. This commit resolves all those inconsistencies. Fixes #680
@ftsamis Can you please review the PR? :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs some more work before it can be merged:
Overall changes suggested:
- Backticks are unnecessary.
- Many docstrings need indentation corrections.
- Some docstrings follow a different (non numpydoc) format. They should be also changed
- Would be great if all descriptions started with a capital letter and ended with a period
.
, but not a real deal-breaker. - I'd suggest checking this table for specifying more complex types (such as parametrized numpy arrays). So instead of
np.ndarray (float)
or similar it should benp.ndarray[float]
@@ -52,7 +52,7 @@ def read_hdf5_data(fname, dset_name): | |||
Returns | |||
------- | |||
|
|||
data : `~astropy.table.Table` | |||
data : `astropy.table.Table` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Backticks are unnecessary. This applies to all other instances, I won't comment them separately.
@@ -137,19 +137,19 @@ def calculate_density_after_time(densities, time_0, time_explosion): | |||
scale the density from an initial time of the model to the | |||
time of the explosion by ^-3 | |||
|
|||
Parameters: | |||
Parameters | |||
----------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a small thing but keep the dashes matching in length the title (i.e. remove one here). There are other instances too.
----------- | ||
|
||
densities: ~astropy.units.Quantity | ||
densities: astropy.units.Quantity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep the space before the :
consistent. There are other instances of this too.
@@ -272,16 +272,16 @@ def calculate_radiationfield_properties(self): | |||
Parameters | |||
---------- | |||
|
|||
nubar_estimator : ~np.ndarray (float) | |||
nubar_estimator : np.ndarray (float) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found out that PyCharm suggests np.ndarray[float]
. Check the link for more info. Would be nice to also fix any other instances that match the table in the link
|
||
Returns | ||
------- | ||
|
||
: ~numpy.ndarray | ||
: numpy.ndarray | ||
array of frequencies | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indentation here is wrong, also no need for the leading :
if there is no name for the return value but only a type. There are more similar instances.
If None, an attempt will be made to read the atomic data | ||
from config. | ||
|
||
Returns | ||
------- | ||
: ~plasma.BasePlasma | ||
: plasma.BasePlasma |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for the leading :
as previously mentioned.
@@ -139,7 +139,7 @@ def advance_state(self): | |||
|
|||
Returns | |||
------- | |||
converged : ~bool | |||
converged : bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Over-indentation
In many places, docstrings violate the numpydoc format, hence breaking
the type hinting in PyCharm. This commit resolves all those
inconsistencies.
Fixes #680