Skip to content

Latest commit

 

History

History
236 lines (154 loc) · 21.7 KB

CONTRIBUTING.md

File metadata and controls

236 lines (154 loc) · 21.7 KB

Contributing to mcpyrate

Code and/or documentation contributions are welcome!

This document is intended to benefit anyone who wishes to understand the mcpyrate codebase and/or documentation better, and/or contribute to the project. We believe the possibility to do so should be available to professional developers, hobby developers, academic researchers (particularly in fields other than CS), and students alike.

The preferred contribution mechanism is a pull request on GitHub. New to GitHub?

Table of Contents

Understanding the code

We follow the mcpy philosophy that macro expanders aren't rocket science. We keep things as explicit and compact as reasonably possible. However, the extra features do cost some codebase size. We also tolerate a small amount of extra complexity, if it improves the programmer UX.

For a clean overview of the core design, look at mcpy, version 2.0.0. Of the parts that come from it, its visitors is our core (the BaseMacroExpander), its core is our expander (the actual MacroExpander), and its import_hooks is our importer. (Its BaseMacroExpander.ismacro method is our BaseMacroExpander.isbound, because that method checks for a raw string name, not an AST structure.) The rest of the module layout should be clear.

Then see our importer. After mcpyrate.activate has been imported, the importer becomes the top-level entry point whenever a module is imported.

See also Understanding the quasiquote system.

Code style

  • Follow PEP8, but not into foolish consistency.

    • Particularly, a blank line (or a double-blank line when appropriate) is a paragraph break, to separate logical units. Just like when writing prose, don't break the paragraph inside a logical unit, no matter what flake8 tells you. This helps make the overall structure explicit and easy to see at a glance.

    • Tools like black are not really appropriate here; a macro expander is as far from an everyday Python project as, say, a language extension is. Prefer to imitate the existing style.

    • Modules vendored from other open-source projects are exempt from code style.

      • This makes them trivial to update, as well as compare to the original version.
      • As of 3.0.0, we have ansi.py from colorama.
  • Follow the Zen of Python. Particularly, simple and explicit is often preferable.

  • Use semantic versioning.

  • Export explicitly.

    • Any names intended as exports must be present in the magic __all__. Although not everyone uses it, this is the official mechanism to declare a module's public API, recommended by PEP8.
    • We don't currently use this for re-export, but it means we could from .somemodule import * in the top-level __init__.py to automatically pull in the public API, and only the public API of somemodule.
  • Prefer small modules, each with a clearly defined area of responsibility.

    • Keep code short, but clear.
      • No golf.
      • Sometimes a small amount of repetition is harmless, vs. the learning cost of one more abstraction.
    • Each source file should be less than ≈700 lines. That's a lot of Python.
      • Going 10%-ish over the length limit is fine. But no 10k SLOC source files like often seen in Emacs plugins.
      • If you're doing simple but tedious things, exceeding the limit by more than 10% may be fine.
        • unparser.py is almost 1000 lines, simply because Python's grammar is so huge. The implementation is pretty much a laundry list of simple instructions for each AST node type. It's clearer if it's all in the same module.
      • If you're doing complex things, stick to the limit, and prepare to comment more than usual.
        • "First, we want to establish the idea that a computer language is not just a way of getting a computer to perform operations but rather that it is a novel formal medium for expressing ideas about methodology. Thus, programs must be written for people to read, and only incidentally for machines to execute. " --Abelson, Sussman & Sussman
        • quotes.py is ≈300 lines of code, ≈500 lines of explanation, to save research and detective work. That's fine.
      • If you're doing a number of related small things, place them in the same module. (Python isn't MATLAB.)
        • For example, utility functions; the area of responsibility can then be the category of use cases for that collection of utilities. We have utils.py for general use, and coreutils.py for things only macro expanders need.
          • If you ever find the need for a moreutils.py due to the length limit, consider if there's a category of use cases that could be split off. If not, just keep extending utils.py.
  • Name clearly.

    • "There are only two hard things in Computer Science: cache invalidation and naming things." --Phil Karlton
    • Get the code working first. Then think of how to best name and organize things to convey your meaning, and aggressively rename (and possibly move to a different module) anything that hasn't hit a published release yet. Maybe several times, until the result is satisfactory.
      • Really, anything: named parameters of functions, functions, classes, modules, even packages.
      • Try to get the names right before publishing a release. This is important.
        • Not getting the names right and then keeping them gave us Common Lisp. terpri? flet? letf? labels?
        • But we don't want to bump from version X.Y to (X+1).0 just because some names needed changing for clarity.
    • Avoid reusing the name of a module for a thing inside that module. Hence e.g. unparser, which provides unparse.
  • For strings, prefer double quotes.

    • Docstrings should use the triple double quote.
  • When defining functions, consider using keyword-only parameters.

    • Remembering argument ordering is really, really hard. Avoid the need.
    • Usually we have just one positional parameter (beside self, if any), sometimes two.
  • When calling functions, pass arguments by name when it improves clarity.

Docstring and comment style

  • Prefer 80-ish columns; going 10% or so over is ok.

    • Prefer to break the limit, if that allows a much better paragraph fill.
  • No need for full-blown rst formatting.

  • Feel free to use plain-text **bold** and *emphasis*.

    • The preferred wording to describe a usage pitfall is **CAUTION**: ...
  • Quote any inline code with backticks: `zorblify` is a callable that....

  • Indent display-style code snippets. Use a double colon to begin the indented block, like in rst:

    """Do stuff.
    
    Example::
    
        c = dostuff(a, b)
    """
  • Avoid bureaucracy.

    • Don't repeat the function signature in the docstring.
      • But do mention the return value (type, and what the value means).
    • No need for section headers such as Parameters:.
      • Just describe the parameters. Or even omit if blindingly obvious.
    • Write as briefly as reasonably possible. But try not to leave out anything important.
  • When there is a conflict between the two, prefer surgical clarity of expression over conventional usage of English.

    • Use unambiguous wording, even if that makes a sentence a bit longer. Optimally, no sentence can be misinterpreted.
    • Be precise. To define a term, choose a word (or phrase) once, and then use it consistently.
      • E.g. functions have parameters, which are filled with arguments at call time. (This is PEP8 usage.)
      • Repetitive use of the same word (or phrase) is a non-issue, but lack of precision causes bugs.
    • Quotes belong around the whole quoted thing, whether or not it contains punctuation, and whether or not punctuation immediately follows the quote.
      • ""Meow, the cat said.", said the docstring." is correct.
    • Splitting an infinitive often improves clarity.
      • "Zorblifying causes the resulting list to not have AST markers..." makes it explicit the not applies locally to the to have, removing the need to scan the whole following phrase.
  • In headings, capitalize the first word only, as well as any proper names. Do not use title case. Combined with font styles, this eases scanning.

    • Don't capitalize a proper name that officially starts with a lowercase letter. This includes functions and their parameters.
      • Sometimes, this leads to a sentence starting in lowercase. That's ok.
      • If there's a convenient and equally clear alternative wording (which still directs the reader's attention to the important thing), feel free to use that. If not, just accept it.

Automated tests

  • Test what the software should do, not how it does it. Avoid testing internal details.

  • Aim at clarity. Don't worry too much if you have to make the tests a little DAMP.

    • That is, prefer Descriptive And Meaningful Phrases over maximal elimination of repetition, because DRY tends to introduce a learning cost. Some repetition is fine, if it lets you omit defining an abstraction that's only needed for those particular tests.
    • But that's not a hard rule, either. For example, if you can fit both the definition and all its uses onto the screen at once, and the result is both shorter and more easily understandable than without making that definition, then feel free to do that.
  • Write tests that can double as usage examples.

    • This yields complete code examples that can be linked to in the user manual, with the machine-checked guarantee that if tests pass, the examples are known to run on the version that was tested.
  • Use common sense to focus test-writing effort where it matters most.

    • Don't bother testing trivial things. For example, a test checking that a constructor assigns the instance attributes correctly is most often a symptom of testing like the TSA, and causes software ossification for no benefit.
    • Test nontrivial implementations, and particularly their interactions. Write tests that can double as advanced usage examples.
    • Aim at testing edge and corner cases, particularly ones that could turn up in practice. Features should be orthogonal, and when not reasonably possible, they should interact sensibly (or alternatively, error out with a sensible message) when used together. Test that they do.

Test framework

The tests are based on a minimalistic no-framework (cf. NoSQL) approach, using assert statements, and a (very short) custom test runner, runtests.py.

The reason mcpyrate does not use a standard test framework is that the standard library's unittest is far too verbose to use (for my tastes), and the otherwise excellent, de facto standard Pytest installs an import hook (to redefine the semantics of assert), so it can't work when a macro expander is in use.

For these same reasons, I have actually developed a macro-enabled test framework, unpythonic.test.fixtures. I would love to refactor it into mcpyrate, but that's a chicken-and-egg problem. The framework requires macros, so it can't be used for testing a macro expander. It also requires conditions and restarts (à la Common Lisp), which is a language extension orthogonal to macros, and so belongs in a separate library. So, currently the framework lives in unpythonic (which, as of 0.15, runs on mcpyrate).

As someone once said, I've never seen a system that bootstraps elegantly. So we test this low-level layer using assert; it's good enough.

The test runner runtests.py

Test discovery is automatic. Test execution order:

  • Test directories are detected top-down, and sorted alphabetically at each level.
    • Any directory, at any level, whose name is test, is considered as a test directory.
  • In each directory, the test modules run alphabetically.
    • Test modules are those .py files whose filenames match the shellglob test_*.py.

Each test module is imported as a Python module (with mcpyrate active), and its runtests function is called, with no arguments. Normal termination indicates that tests passed, and the runner will report so. On AssertionError it will report a test failure, and on any other uncaught exception, an error.

NOTE: When examining test results, check first that test_quotes and test_compiler pass. Most test cases in the mcpyrate codebase rely on these features, so if they don't work properly, likely nothing else will either.

Demos run after the tests, sorted top-down alphabetically, just like the tests. Demos are run automatically to ensure that the demo examples are sufficiently up to date in order not to crash on the version of mcpyrate being tested.

Demos may use asserts similarly to tests. This may be beneficial to improve the human-readability of expectations regarding what the demo example is supposed to do, and is recommended.

Demos live in the demo/ subfolder of the project top level. Each demo is either:

  • A directory with an entry point, demo.py.
    • The entry point runs as a Python script, as if it was invoked as macropython demo.py.
    • Any other files and subdirectories can be used by the demo itself for whatever purposes it needs to.
      • Some demos may have a run.py for running them with bare python3; it is also ignored.
  • A single .py file, possibly in the same directory with other single-file demos.
    • In this case subdirectories are automatically scanned for more demos.

During demos, any uncaught exception is reported as a failure.

Layout

As of 3.1.0, tests are contained inside the mcpyrate package, in mcpyrate.test, but they are not installed by setup.py. This goes against the commonly accepted wisdom on Python project directory layout: Pytest's recommendations, and IonelMC's famous blog post.

Tests are not installed simply because a typical use site does not need them; by the time mcpyrate is installed, it has already been tested. As for the tests living in a subpackage, for me this hasn't mattered that much in practice. The original motivation was to be able to change the project name easily during early development (using only relative imports), but that's moot by now.

The important feature is to support some form of in-tree testing.

As of 3.1.0, the tests are placed under a test subpackage of the package being tested, and the test modules use relative imports. The REPL subsystem, mcpyrate.repl, currently has no automated tests associated with it. If added later, those should be placed in mcpyrate.repl.test.

Individual test modules are expected to be invoked from the project top level as python3 -m mcpyrate.repl.macropython -m mcpyrate.test.test_somemodule. Here the first -m goes to python3, whereas the second one goes to macropython. Note this explicitly invokes the in-tree macropython, instead of an installed copy (if any).

To run all tests, at the project top level, python3 runtests.py.

A future possibility is to change the project structure to use a separate test folder, and use absolute imports in the test modules. By invoking the test runner as a script from the project top level, its containing directory will end up as the topmost entry on sys.path, thus resolving those absolute imports as pointing to the source tree (instead of to an installed copy, if any).

Why in-tree testing matters

Quite simply, it allows quick testing of changes to the codebase without installing it, thus speeding up the edit-run cycle.

I think a fast edit-run cycle is crucial. If this cycle is slow, it encourages the bad practice of making many unrelated changes at once, to amortize the (perceived) waiting cost at the run step.

By "fast", I mean that the run step (including compilation!) should complete in a couple of seconds at most, preferably less. Keep in mind that one second is Nielsen's oft-quoted limit for avoiding interruption of the thought process. In my opinion that's not too much to ask; software development does not need to be that CPU-hungry. Using available machine resources intelligently, it's perfectly achievable in many cases to invoke a relevant testset, and get the results in less than a second.

In my opinion, this level of performance should be the norm. Near-realtime feedback is crucial if the computer is to act as an intelligence amplifier. It's all about the kind of conversation you can have with your ideas as you elaborate on them, down to the level of detail required to make them machine-executable. Being able to run everything in-tree facilitates, in effect, a face-to-face conversation with the computer. The powerful thing is, you don't need a fully formed idea before you present it to the computer; you can sketch, and experiment on the smallest details, and the machine will respond near-instantly. This is a particularly nice workflow for building any non-trivial design.

This is also the reason why the test modules are designed to be executable individually. There is no point in running the whole (potentially time-expensive) test suite right away, until the module being worked on passes its own unit tests. The full test suite is just the second step of verification.

(A variant of the different kind of conversation remark was originally famously made by Paul Graham, in On Lisp. See relevant quotation.)

Why other forms of testing matter too

The drawbacks of in-tree testing are well known:

  • A developer's machine is far from a pristine installation environment. There may be subtle but important differences between what the developer thinks they have versus what they actually do have; e.g. library versions may differ from the expected ones, or additional libraries may be present.
  • There is no guarantee that the package installs correctly, or at all, because the install step is not tested.
  • There is no guarantee that the installed copy runs correctly, or at all. The project folder may contain important files that are accidentally omitted from the install, or in extreme cases, even from version control.

All three can be addressed by complementing in-tree testing with some form of CI. As of mcpyrate 3.3.0, a CI workflow was added to do this; it runs automatically on any commit to master.

However, as of 3.5.3, test coverage is much less than optimal; this is tracked in issue #2. While extending tests, we must consider which parts of the library are stable enough to justify drawing a semi-permanent interface line, because introducing tests tends to ossify the design of the system being tested.