-
Notifications
You must be signed in to change notification settings - Fork 26
Coding guidelines
Apart from the requirements (already stated in the ARAX Maintenance SOP) document regarding static code checks for PEP8 compliance, type checking, and linting, there are some additional guidelines for coding changes on the ARAX code-base:
- If you are adding or modifying a line of code for debugging that will need
to be removed or updated before you commit, make sure you add a line comment
# :DEBUG:. This is important so you can ensure that debugging code doesn't leak through into the commit. - Please use modern type hints (from python3.10 or newer), so no uppercase
DictorList, and make sure to use| Noneinstead ofOptional. - Please review your code with a code LLM (e.g., Claude code or Copilot) to check for bugs before committing. Best results from LLM review seem to be when the LLM can see the code diffs in the context of the full ARAX code-base.
- Please visually inspect every line of your code diffs with
git diffbefore committing; this is the perfect opportunity to catch debug code (which would be indicated with a# :DEBUG:line comment) before it gets accidentally committed. - If you update the code in a function, it is best practice to bring it up
to modern standard in terms of type hinting and PEP8 formatting. But be
cautious about entirely reformatting (such as with
blackor whatever) a whole module for which another active-contributing team member has the vast majority of commits; check with her/him before making a major reformat. - If your code update introduces a new dependency on a PyPI distribution
package, it is critical that you update the ARAX
RTX/requirements.txtfile with the version-pinned dependency. You then need to test the updatedrequirements.txtfile, usingvenv/bin/pip install -r RTX/requirements.txt, to make sure it works as modified. If your code update introduces a dependency on a mypy types package liketypes-requestsor whatever, you need to put that inRTX/dev-requirements.txtas a pinned dependency and test it withpip. - All newly contributed python code from the OSU team should adhere to the PEP8 Style Guide for Python Code. Among other things, this means: No hard tabs; four space indentation; max 79 characters per line; proper use of CamelCase or snake_case or CAPS_SNAKE_CASE; proper use of whitespace around infix operators; etc.
- Where possible, try to minimize use of "path surgery", i.e.,
sys.path.append. Some path surgery is inevitable given the bifurcation of the code-base intoRTX/code/UI/OpenAPI/python-flask-serverandRTX/code/ARAXsubtrees, but please try to minimize it, bearing in mind that ARAX is run usingpython -mso it should be able to interpret dotted imports likefrom openapi_server.models.node import Node. - Lazy module import is to be avoided if at all possible, since ARAX runs in a multithreaded environment. This avoids nasty race conditions in the context of circular imports (see below), which is probably the main reason lazy import is used. It is strongly preferred to eagerly import modules when the application server is starting up in single-threaded mode. See ARAX issue 2788 for details.
- Circular module imports, while sometimes necessary, should be avoided if possible (requires at least one lazy import, and see item 1 above).
- Please do not use hard tabs in your code. This can be avoided by configuring your editor to insert spaces as needed for proper indentation. See PEP8. Per PEP8 guidelines, for python code, please use four spaces per indentation level. Consider configuring your editor to visually flag PEP8 compliance issues; the resulting code will be easier for others to work with, whose editors flag PEP8 issues.
- Please try to use PEP 484 type hints wherever possible in your Python code.
- No secrets are to be checked into the project's GitHub code repository. (AWS keys, database passwords, ssh keys, etc.)
- If you add a dependency on a python distribution package in some module, generally you
should add that package to the top-level
requirements.txtfile or the top-leveldev-requirements.txtfile, if it is not already there (thedev-requirements.txtis for developer tools likepytest,mypy,pylint, andvulture). - When coding a new feature or fixing a bug, if possible, please consider
adding a
pytestunit test to a module (either a new module or an existing module) inRTX/code/ARAX/test. - ARAX uses CPython 3.12 in production; any newer language features should be avoided or used with utmost caution (i.e., only if you fully understand the implications of that choice for deployment).
- Only LF (i.e.,
\n) line termination for text files in the repo. Please do not commit text files with Windows line termination (CRLF) unless you have configured git to automatically convert them to LF line termination. - Please try to format your code to PEP8 formatting conventions where possible. The exception is line length, where the PEP8 standard limit (79 characters) is unreasonably short given the capabilities of modern displays; but please do at least try to keep lines to a less than 120 characters, if possible.
-
Don't parse YAML or large JSON files at query time. In fact, try to avoid reading any configuration files from the filesystem at query time, if you can. Config files are generally small and can be easily cached in memory; in most cases, you're better off loading them at application start-up.
-
Don't retrieve files via
scpor similarly slow mechanisms, at query time. -
Don't poll in a loop without using a wait mechanism like
time.sleep. -
Avoid using
subprocess.runorsubprocess.check_callto perform a task (e.g., moving or removing a file) that can be done using a function in theosorsystemnamespace. -
Don't use cross-AWS-region database querying. It is very slow.
-
Don't run the ARAX database manager at query time.
-
Don't run
kp_info_cacher.refresh_kp_info_cachesat query time. Only the ARAX Background Tasker should be doing that. -
Don't do a task a query time that can (without undue complexity) be done at application start-up or by the ARAX Background Tasker.