Skip to content
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

fix: consistent doc string wrapping #116

Merged
merged 23 commits into from
Sep 10, 2024
Merged

fix: consistent doc string wrapping #116

merged 23 commits into from
Sep 10, 2024

Conversation

beckermr
Copy link
Collaborator

@beckermr beckermr commented Sep 9, 2024

This PR ensures we are using consistent doc string wrapping via the implements decorator.

I have only wrapped public functions/methods except for a few cases where things were private but clearly meant to be more public facing.

closes #71

Comment on lines 32 to -35
def __init__(self, value):
"""
:param value: The measure of the unit in radians.
"""
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The init should not have a doc string per normal python conventions.

@beckermr beckermr changed the title fix: consistent doc string wrapping for angle.py fix: consistent doc string wrapping Sep 9, 2024
Copy link

codspeed-hq bot commented Sep 9, 2024

CodSpeed Performance Report

Merging #116 will degrade performances by 30.89%

Comparing doc-string-wrapping (68714f5) with main (48bf965)

Summary

⚡ 1 improvements
❌ 2 regressions
✅ 8 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main doc-string-wrapping Change
test_benchmarks_lanczos_interp[kval-conserve_dc-run] 446.9 µs 646.6 µs -30.89%
test_benchmarks_lanczos_interp[kval-no_conserve_dc-run] 446.4 µs 345.4 µs +29.24%
test_benchmarks_lanczos_interp[xval-no_conserve_dc-run] 731.7 µs 928.3 µs -21.17%

@@ -213,15 +207,8 @@ def xValue(self, *args, **kwargs):
pos = parse_pos_args(args, kwargs, "x", "y")
return self._xValue(pos)

@implements(_galsim.GSObject._xValue)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few formally private but clearly public-facing methods like this one that I went ahead and wrapped.

@beckermr beckermr marked this pull request as ready for review September 10, 2024 04:07
Copy link
Collaborator

@ismael-mendoza ismael-mendoza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few small changes, otherwise looks great! Thanks, this made the codebase much smaller

jax_galsim/interpolatedimage.py Outdated Show resolved Hide resolved
jax_galsim/interpolatedimage.py Outdated Show resolved Hide resolved
jax_galsim/interpolatedimage.py Outdated Show resolved Hide resolved
beckermr and others added 3 commits September 10, 2024 15:45
Co-authored-by: Ismael Mendoza <[email protected]>
Co-authored-by: Ismael Mendoza <[email protected]>
Co-authored-by: Ismael Mendoza <[email protected]>
@beckermr beckermr enabled auto-merge (squash) September 10, 2024 14:00
@beckermr beckermr merged commit 047cf19 into main Sep 10, 2024
7 checks passed
@beckermr beckermr deleted the doc-string-wrapping branch September 10, 2024 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

consistent wrapping for docstrings
2 participants