Skip to content

Add class docstring for DiffractionObject #265

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

Merged
merged 3 commits into from
Dec 23, 2024

Conversation

bobleesj
Copy link
Contributor

Closes #240 - DiffractionObject docstring do over

Copy link

codecov bot commented Dec 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.68%. Comparing base (ce6c9cc) to head (90672ee).
Report is 21 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##              main     #265      +/-   ##
===========================================
- Coverage   100.00%   98.68%   -1.32%     
===========================================
  Files            8        8              
  Lines          404      379      -25     
===========================================
- Hits           404      374      -30     
- Misses           0        5       +5     

see 4 files with indirect coverage changes

@bobleesj
Copy link
Contributor Author

bobleesj commented Dec 22, 2024

@sbillinge ready for review - cc'ing @yucongalicechen

Copy link
Contributor

@sbillinge sbillinge left a comment

Choose a reason for hiding this comment

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

this is fantastic! Few small comments inline then we can merge.

@@ -36,50 +36,38 @@ def _setter_wmsg(attribute):

class DiffractionObject:
"""
Initialize a DiffractionObject instance.
A class to represent diffraction data for various scientific experiments involving scattering
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use the numpy standard. First line is <80 chars and gives a high level statement of the purpose.

Then a blank line. then a more meaty description such as you have here).

Copy link
Contributor Author

@bobleesj bobleesj Dec 22, 2024

Choose a reason for hiding this comment

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

done - do we want to use automatic doc formatting in pre-commit?

  # docformatter - formats docstrings in python code
  - repo: https://github.com/s-weigand/docformatter
    rev: 5757c5190d95e5449f102ace83df92e7d3b06c6c
    hooks:
      - id: docformatter
        additional_dependencies: [tomli]
        args: [--in-place, --config, ./pyproject.toml]

??

This basically modifies our code to PEP 257, although it does not fix the length, it does change the format.

REF: https://github.com/PyCQA/docformatter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screenshot 2024-12-22 at 3 20 30 PM

Like black, it first fails, and the modify it automatically for us.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(i can create a separate PR for this)

@bobleesj
Copy link
Contributor Author

@sbillinge ready for review - some comments are "outdated" and no longer displayed under Files changed but please refer to my latest replies above

@bobleesj
Copy link
Contributor Author

(fixed a quick typo)

@sbillinge sbillinge merged commit 8107874 into diffpy:main Dec 23, 2024
4 of 5 checks passed
@bobleesj bobleesj deleted the class-docstring branch December 23, 2024 19:38
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.

DiffractionObject docstring do-ove
2 participants