nhsdigital / rap_example_pipeline_python Goto Github PK

          I recommend using type definitions in function definitions. It means the code is more self-documenting and can really aid development

def configure_logging(log_folder: Path, config: Dict[str, Any]) -> None:

This changed would be something that should be propegated to the rest of the repository, if it is a standard you want to adopt.

Originally posted by @josephwilson8-nhs in #6 (comment)

use Path objects from the pathlib library for all paths so they will be correct on different OSs

          We need to get rid of this whole "adapting for your project" section

Originally posted by @SamHollings in #6 (comment)

Add logging to the new sections we've added

          Have you checked the formatting off all other docstrings in the repo, do they match this style?

Originally posted by @josephwilson8-nhs in #6 (comment)

          ah we should probably remove this conda section

Originally posted by @SamHollings in #6 (comment)

Hi, I am getting the following error:

Java not found and JAVA_HOME environment variable is not set.
Install Java and set JAVA_HOME to point to the Java installation directory.

Do I need Java to be installed? I have done:

python -m pip install -r requirements.txt

Add tests where possible:

• unit tests to cover the individual functions
• backtests as an end-to-end test

If making the test is too difficult, just add an "empty" placeholder - to show that we at least are aware it needs testing.

Use Pytest as the framework - see the examples.

Update imports to import whole module as alias and update function references throughout.

A la Joe's suggestion:

          This is more of a personal style choice, but I like to import the file under an alias, rather than the function

from src.utils import file_paths as paths, logging_config as logs, spark
from src.data_ingestion import get_data as data, reading_data as read
from src.processing import aggregate_counts as agg
from src.data_exports import write_csv as write

You will need to change function calls e.g. get_config will become paths.get_config, but it will be more clear where the function is from

Originally posted by @josephwilson8-nhs in #6 (comment)