|
1 | 1 | # Contributing to python-nnf |
2 | 2 |
|
3 | | -More information coming soon. |
| 3 | +## Running the tests and linters |
4 | 4 |
|
5 | | -## Notes for now |
| 5 | +[`tox`](https://tox.readthedocs.io/en/latest/) is used to run the tests and linters. After installing it, run: |
6 | 6 |
|
7 | | -- `tox -e py3` |
8 | | -- Set notation for Internal nodes (rather than lists) |
9 | | -- Use of `@memoize` |
10 | | -- Use of Typing / mypy |
| 7 | +``` |
| 8 | +tox -e py3 |
| 9 | +``` |
| 10 | + |
| 11 | +This will install and run all the tooling. |
| 12 | + |
| 13 | +## Mypy |
| 14 | + |
| 15 | +[Mypy](https://mypy.readthedocs.io/en/stable/) is used for static typing. This is also managed by `tox`. |
| 16 | + |
| 17 | +You can look at the existing code for cues. If you can't figure it out, just leave it be and we'll look at it during code review. |
| 18 | + |
| 19 | +## Hypothesis |
| 20 | + |
| 21 | +[Hypothesis](https://hypothesis.readthedocs.io/en/latest/) is used for property-based testing: it generates random sentences for tests to use. See the existing tests for examples. |
| 22 | + |
| 23 | +It's ideal for a feature to have both tests that do use Hypothesis and tests that don't. |
| 24 | + |
| 25 | +## Memoization |
| 26 | + |
| 27 | +[Memoization](https://en.wikipedia.org/wiki/Memoization) is used in various places to avoid computing the same thing multiple times. A memoized function remembers past calls so it can return the same return value again in the future. |
| 28 | + |
| 29 | +A downside of memoization is that it increases memory usage. |
| 30 | + |
| 31 | +It's used in two patterns throughout the codebase: |
| 32 | + |
| 33 | +### Temporary internal memoization with `@memoize` |
| 34 | + |
| 35 | +This is used for functions that run on individual nodes of sentences, within a method. For example: |
| 36 | + |
| 37 | +```python |
| 38 | + def height(self) -> int: |
| 39 | + """The number of edges between here and the furthest leaf.""" |
| 40 | + @memoize |
| 41 | + def height(node: NNF) -> int: |
| 42 | + if isinstance(node, Internal) and node.children: |
| 43 | + return 1 + max(height(child) for child in node.children) |
| 44 | + return 0 |
| 45 | + |
| 46 | + return height(self) |
| 47 | +``` |
| 48 | + |
| 49 | +Because the function is defined inside the method, it's garbage collected along with its cache when the method returns. This makes sure we don't keep all the individual node heights in memory indefinitely. |
| 50 | + |
| 51 | +### Memoizing sentence properties with `@weakref_memoize` |
| 52 | + |
| 53 | +This is used for commonly used methods that run on whole sentences. For example: |
| 54 | + |
| 55 | +```python |
| 56 | + @weakref_memoize |
| 57 | + def vars(self) -> t.FrozenSet[Name]: |
| 58 | + """The names of all variables that appear in the sentence.""" |
| 59 | + return frozenset(node.name |
| 60 | + for node in self.walk() |
| 61 | + if isinstance(node, Var)) |
| 62 | +``` |
| 63 | + |
| 64 | +This lets us call `.vars()` often without worrying about performance. |
| 65 | + |
| 66 | +Unlike the other decorator, this one uses `weakref`, so it doesn't interfere with garbage collection. It's slightly less efficient though, so the temporary functions from the previous section are better off with `@memoize`. |
| 67 | + |
| 68 | +## Documentation |
| 69 | + |
| 70 | +Methods are documented with reStructuredText inside docstrings. This looks a little like markdown, but it's different, so take care and look at other docstrings for examples. |
| 71 | + |
| 72 | +Documentation is automatically generated and ends up at [Read the Docs](https://hypothesis.readthedocs.io/en/latest/). |
| 73 | + |
| 74 | +To build the documentation locally, run `make html` inside the `docs/` directory. This generates a manual in `docs/_build/html/`. |
| 75 | + |
| 76 | +New modules have to be added to `docs/nnf.rst` to be documented. |
| 77 | + |
| 78 | +## Style/miscellaneous |
| 79 | + |
| 80 | +- Prefer sets over lists where it make sense. For example, `Or({~c for c in children} | {aux})` instead of `Or([~c for c in children] + [aux])`. |
0 commit comments