use python 3.6 typing #33

orcutt989 · 2020-01-10T09:33:47Z

all variables and functions are typed in order to help with refactoring
fix #31

Description

Types of Changes

Issues Fixed or Closed by This PR

#31

Checklist

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
I have tested the changes and verified that they work and don't break anything (as well as I can manage).

all variables and functions are typed in order to help with refactoring fix mre#31

mre · 2020-01-10T12:40:43Z

Amazing! That was fast. 🚀
Looks like flake8 doesn't like our type hints. Maybe there's an easy fix?

timelapse/__main__.py

timelapse/encoder.py

timelapse/__main__.py

timelapse/notify.py

mre · 2020-01-10T21:53:31Z

timelapse/encoder.py

        # Call ffmpeg with settings compatible with QuickTime.
        # https://superuser.com/a/820137
-        command = ["/usr/local/bin/ffmpeg", "-y",
+        command: List[str]= ["/usr/local/bin/ffmpeg", "-y",


Guess this needs a

from typing import List

import to work properly.

Done 👍
db3330a

mre · 2020-01-10T21:59:55Z

timelapse/recorder.py

                str(get_screen_with_mouse_index() + 1), '-t', self.format, filename],
-            check=True)
-        self.screenshot_counter += 1
+            check: bool =True)


This is a parameter that we pass to subprocess.run. check=True is equivalent to writing True in this case. We just use a named parameter here because it's more readable for 👨 👩. Guess we have to remove this type hint to make it syntactically correct.

Done 👍 bb76c7c

orcutt989 · 2020-01-10T23:22:43Z

Thoughts @mre https://stackoverflow.com/questions/43187829/is-there-a-way-to-make-flake8-check-for-type-hints-in-the-source?

Type hints are only allowed when a new variable is defined.

mre · 2020-01-12T00:26:27Z

Skipped some more type hints for reassignments.
Type hints are only allowed on when a new variable gets introduced. 😉
Think this PR is ready to get merged! 🎉
Thanks for pushing this forward @orcutt989. timelapse has types thanks to you!

orcutt989 · 2020-01-12T03:07:09Z

Wooo! Thanks for letting me help!

estysdesu · 2020-03-13T18:29:35Z

Does it make sense at this point to add mypy to the dev-dependencies now for type checking? Or is there some other way these are being checked?

Mypy would greatly help catch things like how a float type was passed to a function argument that is typed as int. It probably should be typed as Union[int, float] or something numeric like such. Although not big for this case as Python can handle that without hitting an exception, mypy also helped detect when passing an Exception type to an argument thats expecting a str which could be a bigger issue (not in this case though).

EDIT: mypy reports multiple typing errors (omitting the library stubs missing as that was expected and can be configured off):

(timelapse-aCV8DQCs-py3.7) bash-5.0$ mv timelapse/__init.py__ timelapse/__init__.py    # see issue #43 
(timelapse-aCV8DQCs-py3.7) bash-5.0$ mypy -p timelapse
timelapse/__main__.py:9: error: Cannot find implementation or library stub for module named 'notify'    # imports should be renamed for packages to import timelapse.notify or from . import notify (https://docs.python.org/3.7/tutorial/modules.html#intra-package-references)
timelapse/__main__.py:10: error: Cannot find implementation or library stub for module named 'encoder'    # see above
timelapse/__main__.py:11: error: Cannot find implementation or library stub for module named 'recorder'    # see above
timelapse/__main__.py:47: error: Name 'NSObject' is not defined    # to satisfy mypy, all of these objects should be explicitly imported (from AppKit import NSObject, NSStatusBar, ...) or use # type: ignore
timelapse/__main__.py:63: error: Name 'NSStatusBar' is not defined
...
timelapse/__main__.py:99: error: Name 'NSMenuItem' is not defined
timelapse/__main__.py:103: error: Name 'NSMenuItem' is not defined
timelapse/__main__.py:110: error: "None" has no attribute "join"    # typing is not present for self.recorder; mypy infered incorrect type because it was set to None; type should be Optional[Recorder]
timelapse/__main__.py:121: error: Attribute 'recording' already defined on line 54    # type defined twice
Found 24 errors in 3 files (checked 5 source files)

I'd be happy to submit a PR that

Adds mypy to the dev dependencies
Cleans up mypy issues

Just let me know if this is something you'd be interested in. Glad to see this project make use of Python typing. I think its really valuable.

mre · 2020-03-13T22:53:26Z

Thanks for looking into this @estysdesu.
A PR is most welcome.
As a start, I've added a Python linter to the CI build process in #45,
but we should still add mypy to the dev dependencies and clean up the issues. 👍

mre · 2020-03-13T23:09:09Z

Your code has been rated at 1.47/10

There's a bit of work to do, I guess. 😆

orcutt989 added 2 commits January 10, 2020 04:30

use python 3.6 typing

9f056b2

all variables and functions are typed in order to help with refactoring fix mre#31

remove list

4833469

mre closed this Jan 10, 2020

mre reopened this Jan 10, 2020

mre reviewed Jan 10, 2020

View reviewed changes

timelapse/__main__.py Outdated Show resolved Hide resolved

mre reviewed Jan 10, 2020

View reviewed changes

timelapse/encoder.py Outdated Show resolved Hide resolved

mre reviewed Jan 10, 2020

View reviewed changes

timelapse/__main__.py Outdated Show resolved Hide resolved

mre reviewed Jan 10, 2020

View reviewed changes

timelapse/notify.py Outdated Show resolved Hide resolved

orcutt989 added 4 commits January 10, 2020 12:34

missing type base_dir fix

59c85f4

add command typing

4e667ab

fix typing of dark_mode

040a50f

fix typing of notify

2785fc7

mre reviewed Jan 10, 2020

View reviewed changes

orcutt989 and others added 4 commits January 10, 2020 18:25

add import for list typing

db3330a

reverted check var typing

bb76c7c

Skip type hints for assignments

a32843f

Type hints are only allowed when a new variable is defined.

Avoid type hints on reassignments

83acf07

Type hints are only allowed when a new variable is defined.

mre merged commit 289d1f3 into mre:master Jan 12, 2020

mre mentioned this pull request Mar 13, 2020

Add linter to Github actions #45

Open

10 tasks

This was referenced Oct 18, 2022

Link Checker Report mre/endler.dev#26

Closed

Link Checker Report mre/endler.dev#27

Closed

Link Checker Report mre/endler.dev#28

Closed

This was referenced Nov 10, 2022

Link Checker Report mre/endler.dev#51

Closed

Link Checker Report mre/endler.dev#52

Closed

Uh oh!

use python 3.6 typing #33

use python 3.6 typing #33

Uh oh!

Conversation

orcutt989 commented Jan 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of Changes

Issues Fixed or Closed by This PR

Checklist

Uh oh!

mre commented Jan 10, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mre Jan 10, 2020

Choose a reason for hiding this comment

Uh oh!

orcutt989 Jan 10, 2020

Choose a reason for hiding this comment

Uh oh!

mre Jan 10, 2020

Choose a reason for hiding this comment

Uh oh!

orcutt989 Jan 10, 2020

Choose a reason for hiding this comment

Uh oh!

orcutt989 commented Jan 10, 2020

Uh oh!

mre commented Jan 12, 2020

Uh oh!

orcutt989 commented Jan 12, 2020

Uh oh!

estysdesu commented Mar 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mre commented Mar 13, 2020

Uh oh!

mre commented Mar 13, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

orcutt989 commented Jan 10, 2020 •

edited

Loading

estysdesu commented Mar 13, 2020 •

edited

Loading