search

napari / napari-animation

A napari plugin for making animations
https://napari.github.io/napari-animation/
Other
76 stars 27 forks source link

Added a pre-commit config file, and ran it on the existing code. #61

Closed Fifourche closed 3 years ago

Fifourche commented 3 years ago

To tackle #56 , I added a config file for pre-commit that I've taken from napari. I modified it by commenting the napari metadata and configuring isort to be compatible with black.

codecov[bot] commented 3 years ago

Codecov Report

Merging #61 (9f076bd) into main (f5f7979) will decrease coverage by 0.21%. The diff coverage is 56.17%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #61      +/-   ##
==========================================
- Coverage   62.74%   62.53%   -0.22%     
==========================================
  Files          15       15              
  Lines         706      710       +4     
==========================================
+ Hits          443      444       +1     
- Misses        263      266       +3
Impacted Files Coverage Δ
napari_animation/_hookimpls.py 100.00% <ø> (ø)
napari_animation/_qt/__init__.py 100.00% <ø> (ø)
napari_animation/_qt/animationslider_widget.py 35.71% <ø> (ø)
napari_animation/_qt/keyframelistcontrol_widget.py 30.43% <0.00%> (ø)
napari_animation/_qt/frame_widget.py 34.88% <10.00%> (ø)
napari_animation/_qt/animation_widget.py 26.44% <35.71%> (+0.61%) :arrow_up:
napari_animation/_qt/keyframeslist_widget.py 27.90% <55.00%> (-0.33%) :arrow_down:
napari_animation/easing.py 69.81% <55.55%> (ø)
napari_animation/animation.py 79.10% <84.00%> (-1.20%) :arrow_down:
napari_animation/__init__.py 100.00% <100.00%> (ø)
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f5f7979...9f076bd. Read the comment docs.

sofroniewn commented 3 years ago

Looking good - though can we now move all our stuff over from setup.py to setup.cfg? Maybe @tlambert03 can advise here too?

Also it looks like all our strings now use " instead of '. I guess that's ok, but i think in napari core we disabled that. I don't have strong feelings though.

@alisterburt, let's see if @tlambert03 weighs in, but then I'd say good for you to merge when you feel ready

Fifourche commented 3 years ago

I moved almost everything from setup.py to setup.cfg (thanks for the info!!) ; I hope I did it right !

Regarding quotes, I checked different files on napari and it seems to be always ' except when there are already some in the string, and they need to be "escaped" ; in this case " is used. Which makes sense to find also in here, as I pretty much took the config from napari! Anyways if it's to be changed I'll find a way to do it :)

alisterburt commented 3 years ago

Thanks for the input @tlambert03 !

@Fifourche - it looks like the string settings are controlled in the pyproject.toml file in napari https://github.com/napari/napari/blob/97865a9051a3ce33ffd58903ea1cb016fd1b4ba9/pyproject.toml#L29-L53

These can probably also be set in setup.cfg, I'm not sure why the isort/black/flake8 settings are divided across setup.cfg and pyproject.toml in napari

tlambert03 commented 3 years ago

These can probably also be set in setup.cfg, I'm not sure why the isort/black/flake8 settings are divided across setup.cfg and pyproject.toml in napari

this part is a bit of a mess. each tool makes its own decisions on what it's going to support, and they're all over the board at the moment. Black only supports pyproject, but some of the others don't support it at all... So there's unfortunately no way yet to have only one file :/

I recommend this article if you're curious: https://snarky.ca/what-the-heck-is-pyproject-toml/

and this: https://blog.pilosus.org/posts/2019/12/26/python-third-party-tools-configuration/

alisterburt commented 3 years ago

Appreciate the links Talley! Looks like everything is moving in the right direction 🙂

alisterburt commented 3 years ago

Thanks @Fifourche for the fixes - I removed some napari specific black config and added you and Talley to the authors list! Feel free to add your name rather than your github handle in a follow-up PR if you would prefer.

Following guidelines from https://black.readthedocs.io/en/stable/pyproject_toml.html about string normalisation for new projects - given that the strings are now all with double quotes anyway I guess we should follow the black style) - let's merge! 🚀

alisterburt commented 3 years ago

hmm, CI now failing on windows - was working 3 days ago with the same testing config in #60

sofroniewn commented 3 years ago

Nice job @Fifourche, thanks for the help also @tlambert03 and great work @alisterburt shepherding this to the finish line!