-
-
Notifications
You must be signed in to change notification settings - Fork 52
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
Updated deprecated test approach + minor changes #89
Conversation
This is my first PR on a public repo and I couldn't find contribution guidelines. I'm happy to revert any changes that do not comply with any guidelines I might be unaware of, especially since I mindlessly ran isort and black over everything. |
Hey @lbreede 👋 Thanks for this PR! I added a section for developers to the readme. That was indeed missing. In general its more convenient to create separate smaller PR's for unrelated changes. That way the "easy" changes can be merged quickly, so changes that need more work or discussion won't hold up the rest. In this particular case I agree with all changes. I don't care much about import sorting, but I don't think it hurts either 😄 |
There are a few linting errors (you can check locally with |
Hi @almarklein, thanks for the warm welcome! I've addressed the linting error and will look into the CI issues. I guess here's another good reason for smaller PRs, I would know where to look 🤦 I'll see if I can get those running again otherwise as you said 😄 |
Thanks for adding the developers section, super helpful and I had actually never used |
Thanks! Looking good! (The pypy build seems to have been broken before this PR.) |
Major changes
with warns(None) as warnings:
with a new syntax by importing the warnings module andwith warnings.catch_warnings(record=True) as warnings_:
. This turned one of the pytest warning to a pytest passed.Minor changes
FFMEG
toFFMPEG
i
with_
for for-loops that do not utilizei
w
->width
,h
->height
,p
->process
Patch changes
isort .
black .