Skip to content

chore: update pre-commit#184

Open
danielsirakov wants to merge 6 commits into
diffpy:mainfrom
danielsirakov:update_pre-commit
Open

chore: update pre-commit#184
danielsirakov wants to merge 6 commits into
diffpy:mainfrom
danielsirakov:update_pre-commit

Conversation

@danielsirakov

@danielsirakov danielsirakov commented May 31, 2026

Copy link
Copy Markdown

@stevenhua0320, ready to review

@stevenhua0320

Copy link
Copy Markdown
Contributor

@danielsirakov same here.

@codecov

codecov Bot commented May 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.63%. Comparing base (a9eae82) to head (8977ae6).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #184   +/-   ##
=======================================
  Coverage   71.63%   71.63%           
=======================================
  Files          25       25           
  Lines        3437     3437           
=======================================
  Hits         2462     2462           
  Misses        975      975           
Files with missing lines Coverage Δ
tests/test_builder.py 100.00% <ø> (ø)
tests/test_characteristicfunctions.py 12.26% <ø> (ø)
tests/test_constraint.py 96.96% <ø> (ø)
tests/test_contribution.py 99.51% <ø> (ø)
tests/test_equation.py 100.00% <ø> (ø)
tests/test_fitrecipe.py 99.76% <ø> (ø)
tests/test_literals.py 99.17% <ø> (ø)
tests/test_objcrystparset.py 10.27% <ø> (ø)
tests/test_profile.py 99.33% <ø> (ø)
tests/test_recipeorganizer.py 99.69% <ø> (ø)
... and 4 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sbillinge

Copy link
Copy Markdown
Contributor

For some weird reason this is deleting blank lines after the docstring. I asked claude and it seems as if this is a bug in v1.7.8 of docformatter which people have requested fixes for. Please could you try again (let's close this PR so we don't merge all the back and forth history) pinning to 1.7.7 and see what happens?

@stevenhua0320

Copy link
Copy Markdown
Contributor

@danielsirakov could you revisit this one as we have an agreement that deleting line form docformatter is a right fix. So, let's do a review to this one and make sure that everything is right. Thanks!

@sbillinge

Copy link
Copy Markdown
Contributor

The files look Ok. We just need to check if this version is passing pre-commit

@stevenhua0320

Copy link
Copy Markdown
Contributor

@danielsirakov Could you take a look for the failure of black and docformatter? Seems there is a conflict in the formatting of the docstring here. If it is the case, we could do a reformatting as before to get around this. Thanks for your effort.

@danielsirakov

Copy link
Copy Markdown
Author

@danielsirakov Could you take a look for the failure of black and docformatter? Seems there is a conflict in the formatting of the docstring here. If it is the case, we could do a reformatting as before to get around this. Thanks for your effort.

Sounds good, I'll take a look after I make the last few edits to diffpy.structure

…rfit into update_pre-commit

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
@danielsirakov

Copy link
Copy Markdown
Author

I was able to bypass the docformatter and black conflict by changing triple quotes to just using double quotes in 3 files. Let me know if this is an appropriate workaround.

@stevenhua0320, ready to review

@stevenhua0320

Copy link
Copy Markdown
Contributor

@danielsirakov I think we have one left for the conflict here:
tests/test_characteristicfunctions.py
you can check what is conflicting by running it on local and see how we gonna fix this.

@danielsirakov

Copy link
Copy Markdown
Author

@danielsirakov I think we have one left for the conflict here: tests/test_characteristicfunctions.py you can check what is conflicting by running it on local and see how we gonna fix this.

I think this had something to do with me merging my edits with the bot edits; I just ran the pre-commit, and it edited the file automatically and passed once I ran it again. I'll push again with the pre-commit's edit.

@danielsirakov

Copy link
Copy Markdown
Author

@stevenhua ready to review

@stevenhua0320

Copy link
Copy Markdown
Contributor

@danielsirakov Still not passing, first you could do the git pull origin <branch-name> to merge the current pre-commit bot update, then do the single black and docformatter sequentially and see what did they edit on the same file.

@danielsirakov

Copy link
Copy Markdown
Author

@danielsirakov Still not passing, first you could do the git pull origin <branch-name> to merge the current pre-commit bot update, then do the single black and docformatter sequentially and see what did they edit on the same file.

Sorry about that, it said it was passing when I ran the pre-commit before pushing so I’m not sure why that’s happening. I’m not home right now, but I’ll take a look as soon as I’m back.

@sbillinge sbillinge left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I left comments. Let's try moving the failing module level docstrings to the top

Exceptions used for SrFit - specific errors.
"""

"Exceptions used for SrFit - specific errors."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This one should be a docstring I think. Does it work putting it right at the top of the file but with triple quotes?

#
##############################################################################
"""Universal import functions for volatile SasView/SansViews API-s."""
"Universal import functions for volatile SasView/SansViews API-s."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should be a docstring, try moving to the top

#
##############################################################################
"""Functions for binding arguments of callable objects."""
"Functions for binding arguments of callable objects."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Docstring

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.

3 participants