ocap discipline aided by static typing #24

dckc · 2016-09-21T05:16:16Z

I'd like to get your take on this approach with static type annotations.

pros:
- provided essential clues for me to get my head around the code
- it should provide auto-complete and the like in intellij (or even emacs, if I bothered to set it up)
cons:
- they represent a maintenance burden
  - the tools are getting better, but this style is not widely adopted in the python community
- They perhaps clutter up the code.
- They can give a false sense of security. For a while I just followed my nose through the mypy error messages with flycheck. With rust or haskell or scala, this is a nearly foolproof approach to refactoring. Not so with mypy, especially since it's silent by default in many cases where it can't tell whether the code is well-typed.

I could perhaps remove them once I'm done doing the ocap style stuff.

Meanwhile, a couple noteable bits of progress:

76eaed3 is an automated bulk-edit: pathlib_edit.py changes open(path, 'rb') to path.open('rb') and os.path.join(self._path, 'configure') to self._path / 'configure' etc. I was OK to do it by hand for the first few files, but my eyes started to glaze over while doing it for catalog.py.
- I tried to rebase my other edits on top of it, but that got unmanageable, so I did a merge. Even that was pretty hairy. I'm somewhat new to git.
- the results are incorrect in the cases such as config.DIR_RESOURCES / filename but I'm not sure how I want to address that config stuff yet.
db99f02 is my first working unit test, building up to a handful of such tests in 0ba5835
- The good news is: applying ocap discipline does result in testable code as intended.
- The bad news is: testing found bugs introduced in refactoring that weren't found by mypy.

edit tool is [pathlib_edit.py][1] Two tweaks in repository.py were done manually. [1]: https://gist.github.com/dckc/40c8caf4c1dc0027ac0d3b1fdbb251d2

- 4 space indentation - 79 characater line length

While get_data(), like import, is actually runtime I/O, we consider access to design-time constants as if it were access to any other static data (string, integer, ...).

- no powerful imports: os, subprocess, ... - copyfileobj is powerless: file access is passed in - PurePosixPath for syntax manipulation is also powerless - use PathExt rather than strings for access to files - allfiles KLUDGE: navigation thru absolute paths - PathTFix.__truediv__ Self type work-around (for p / sub) - cast .iterdir() result because our Self type work-around isn't smart enough - open() -> path.open() - pass urlopen, subprocess, random to Distfile as a distfile.Access - simplify, export UrlopenFn: always returns BinaryIO - util.RunCommand class represents subprocess interface - declare type for util.diff mkPopen arg - Distfile.__init__() static types (e.g. Hash) help with refactoring - complete static types for all Distfile methods - log.warning() is only declared to take a str. hm.

- DiffCreator gets subprocess - BuildDirectory gets platform - FileHandle etc. get an Access (subprocess etc.) - add check_output to RunCommand interface - Builder interface common to HostBuilder and TargetBuilder - refactor allpath KLUDGE using PathExt.platform() and config.config_path_fn - use single-assignment idiom for tar vs bsdtar - prune resource_directory - restore DIR_RESOURCES but only as a PurePath for use with check_call() - Distfile needs a RandomT - never mind PathTFix generic Self It didn't work -- turned into Any -- and hid a few type errors.

- static types rather than **kwargs for op_distfile etc. - NamedTuple rather than dict for deferred package info - types for version (sometimes a class), checksum (always str) - class AnyVersion reifies type common to SimpleVersion, FullVersion - master_sites, patches are Set[str], not List[str] - move `build_depends = set()` to else branch to help mypy - clarify builder.Access name - prune more resource_directory dead code

- two blank lines before class, def - 4 space indent - 79 character line limit

- unused: SimpleVersion, lib_depends, files - two lines before class - spaces around / and after : - }); formatting

run with `py.test`, for example

found through unit testing

add __init__.py for use with pkgutil.get_data perhaps a bit of a kludge? but a harmless one

and move test_suite into `src` dir

EdSchouten · 2016-09-23T09:48:56Z

I like statically typed languages a lot. I was actually not aware that Python supported this kind of stuff. That said, this is something that needs explicit tooling to validate, right? On my Mac:

Python 3.5.2 (default, Aug 16 2016, 05:35:40) 
[GCC 4.2.1 Compatible Apple LLVM 7.3.0 (clang-703.0.31)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> def a(x: int, y: int) -> int:
...     return x + y
... 
>>> a([1, 2], [3, 4])
[1, 2, 3, 4]

We should eventually consider writing some kind of hook for this to disallow changes that break the typing.

Anyway, you've got the green light from my side to work on this. I'd love to get something like this upstreamed. Feel free to come up with a set of changes that only include the static typing (no style changes or ocap related changes) and I'll make sure it lands. Thanks for working on this!

dckc · 2016-09-23T13:42:48Z

The type declarations are indeed ignored at runtime. Static type-checking is done by other tools such as mypy.

Adding static type declarations in a separate branch... hmm... I guess I could try that, though I couldn't test them, and recent experience shows I don't get them right without testing. The ocap changes are necessary for unit testing.

dckc · 2018-01-01T21:28:38Z

I hope to try this out:

Let your code type-hint itself: introducing open source MonkeyType

dckc added 29 commits September 19, 2016 22:07

bulk edit: os.path(), open() -> pathlib API

76eaed3

edit tool is [pathlib_edit.py][1] Two tweaks in repository.py were done manually. [1]: https://gist.github.com/dckc/40c8caf4c1dc0027ac0d3b1fdbb251d2

src.util static type hints in preparation for ocap refactor

676dc70

define pathlib.Path subclass with shutil methods

7e0c2d9

explicit Path authority in util.copy_file, etc.

a7e8cc0

pass web access to unsafe_fetch explicitly

49eb9eb

pass Popen authority to patch() explicitly

dafbb5d

src/rpm: conventional (PEP8) whitespace style

be5b212

- 4 space indentation - 79 characater line length

use pkgutil.get_data for config resources

b657d13

While get_data(), like import, is actually runtime I/O, we consider access to design-time constants as if it were access to any other static data (string, integer, ...).

make the src dir into a package (helps with mypy)

ed67148

src.package: static types, explicit authority

583b758

prune dead "Skip directory names." code

4427e05

add access to /usr andn /usr/local to platform

32160de

src.catalog: ocap discipline, static types (WIP)

78fa349

distfile: PEP8 style

26d3597

- two blank lines before class, def - 4 space indent - 79 character line limit

src.package: prune unused imports

1c8d9b0

package: style: 2 blank lines before class

3c60d52

src/catalog: style tweaks; prune unused items

e40cb5b

- unused: SimpleVersion, lib_depends, files - two lines before class - spaces around / and after : - }); formatting

build_packages: use parens for long import

12dc43c

Merge static_types_mypy into py_style, pathlib

0b61347

some static types for src.version

ee53b09

string-ize fwd refs to types to avoide NameError

a4c8c2e

test_get_target_packages(), trivial case

db99f02

run with `py.test`, for example

walk fix: not is_dir() does not imply is_file()

444094a

fix scope of random in get_starget_packages

b079ac7

found through unit testing

declare packages/** as design-time test data

75c2b84

add __init__.py for use with pkgutil.get_data perhaps a bit of a kludge? but a harmless one

factor mock_io out of test_suite

0ba5835

and move test_suite into `src` dir

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ocap discipline aided by static typing #24

ocap discipline aided by static typing #24

dckc commented Sep 21, 2016

EdSchouten commented Sep 23, 2016

dckc commented Sep 23, 2016 •

edited

Loading

dckc commented Jan 1, 2018

ocap discipline aided by static typing #24

Are you sure you want to change the base?

ocap discipline aided by static typing #24

Conversation

dckc commented Sep 21, 2016

EdSchouten commented Sep 23, 2016

dckc commented Sep 23, 2016 • edited Loading

dckc commented Jan 1, 2018

dckc commented Sep 23, 2016 •

edited

Loading