Peer review as of 2024-02-22T13:45+02:00
Overall
Nice job! More work is surely needed, but direction is correct as is the journey so far.
Some observations
Installation
'poetry install'
Poetry needs by default access to ones passwords. Consider protecting fellow students and giving PYTHON_KEYRING_BACKEND=keyring.backends.fail.Keyring poetry install
as install command. Add --no-root
switch also as now poetry gives an error The current project could not be installed: No file/folder found for package tiralabra
as there is nothing to install in project itself at this time; only dependencies.
Requirements
Project needs quite recent python version. Although 3.12 is the latest stable Python version not every major distro have adopted it yet. distrowatch Nevertheless project seems to work nicely with 3.11 as stated in README.md after minor modification on index.py
. print(f"{"Input:":25...
vs print(f"{'Input:':25...
etc.
Consider if poetry.lock
is necessary. At least it prevented me to do the install dependencies properly. Should project be stuck library versions in poetry.lock
or is pyproject.toml
adequate?
Directory structure
Docs are under hidden .github folder? Why so? poetry build
only includes src/
folder as default.
Although project is small consider using __init__.py
files so that for example from classes import ShuntingYard, Tokenizer
works.
Code and pylint
Code is easy to read. Functions and variables are named reasonably. Comments are great but some places unnecessary like: # if token is a right parenthesis
, if token == ")":
.pylintrc
is clean, but there's quite many tests disabled globally. Preferably get rid of .pylintrc
altogether and specify locally what tests to disable and why.
Tests
UnitTests that are ready seems relevant. All tests passes. Dev tools under 'poethepoet' seems nice choice.
Documantation
Documentation is easy to follow and at least I have feeling that I understand whats going on.
Fuctionality
All that there is to test seems to work. Is 3+++4
supposed to give an error? Also 3^4,4+2
? Maybe some input validation is needed.
Final thoughts
Sorry to point out so many consideration. All in all project is really good on it's way and looks professional. 10 points!