Update python 3.14#186
Conversation
…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.
…rfit into update_pre-commit
…rfit into update_pre-commit
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #186 +/- ##
==========================================
+ Coverage 71.63% 71.71% +0.08%
==========================================
Files 25 25
Lines 3437 3437
==========================================
+ Hits 2462 2465 +3
+ Misses 975 972 -3
🚀 New features to boost your workflow:
|
|
We finally made it, thanks @danielsirakov but this is a horribly dirty PR at this point with lots of nasty merge commits from work done by the bot. Could we maybe redo this on a clean branch in just a few commits? |
|
Sounds good, I'll redo it |
cadenmyers13
left a comment
There was a problem hiding this comment.
@sbillinge @danielsirakov See inline. The way docformatter 1.7.8 works can make some of the docstrings display poorly. Although, this doesn't affect the mouse-over view for users.
There was a problem hiding this comment.
This edit is made by docformatter
There was a problem hiding this comment.
Another docformatter edit that looks odd
There was a problem hiding this comment.
I would prefer if we didn't have to add these FIXME lines. Why not just use docformatter v1.7.7? This just adds technical debt and v1.7.7 works fine enough
Context: This line is added just to prevent black and docformatter conflicts. See this convo for reference: #184 (comment)
There was a problem hiding this comment.
My understanding was that there is no python 3.14 support for 1.7.7
There was a problem hiding this comment.
@sbillinge yes I discovered this. I'm wondering if theres a better way to handle this than just adding the extra lines but this might be fine for now
There was a problem hiding this comment.
We decided this was the least technical debt. We want to keep using docformatter and we want to push out 3:14 and all the other options were way more TD
|
@danielsirakov The pre-commit bot looks to be making lots of edits on your PR. This likely means you don't have pre-commit set up locally. To set it up locally,
This will run pre-commit every time you make a commit. This makes life a lot easier for you too because you will not have to deal with differences in your remote branch and local branch 😅 . Let me know if you need help with anything! |
|
Hi Caden, I redid this as a new PR (#187), which got merged to main as this one was carrying over my long and messy history from a previous PR (#184). I shouldn't have left this PR open after doing that, so sorry for any confusion. Also, I was able to figure out the issues with the pre-commit bot's edits, but I still appreciate the help : ) I'll check if the odd docformatter edits are still there in the version that got merged to main |
|
@danielsirakov Okay no problem. Let me know what you find. Can you please close this then? |
|
I ended up finding that the weird stuff docformatter was doing is still present in the version that got merged to main. We had a lot of issues with docstrings (especially with triple quotes) and docformatter in PR (#184), and we had a couple of workarounds and solutions in that PR as well. I could double-check if this is something that we have a known solution for or if this is a new issue later today, and we could figure out the best approach from there. |
|
Please see my comments on Slack about docformatter. @cadenmyers13 we basically went through the whole thought process you are having wrt the docformatter update earlier in the summer when you were away and arrived back to where we are now. |
|
@sbillinge I saw it. thanks! its clear to me. |
|
@sbillinge I am going to try to spend some time (might have to be next week) getting |
@stevenhua0320 ready to review