Skip to content

Conversation

@bobleesj
Copy link
Contributor

Closes #240 - DiffractionObject docstring do over

@codecov
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.

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