Follow-up to the improvements PR

[sr-murthy]

Follow-up to ARC improvements PR to partially revert some changes that affected schema and parser file generation.

ARC package now includes schemas/ and units/, and can be tested with:

$ make all
...
$ tar tvf dist/isaric_arc-0.1.1-py3-none-any.whl
...
$ tar tvf dist/isaric_arc-0.1.1.tar.gz
...

[pipliggins]

I can't see anything concerning about the units folder moving.

I didn't get a chance to review the previous PR so apologies, this is hangover from there I think - but the changes I can see in isaric_transformations.py in particular are a bit concerning to me.

While I absolutely understand the desire to have the missing/valid data codes defined centrally, isaric_transformations.py is a file which ADTL requires to run with the ARC parsers. As noted in the adtl docs, files containing these additional transformations will not run if using libraries which are not part of the standard lib (or one of adtl's own dependencies) - which is what these changes have done by creating a dependency on the ARC package. ADTL won't run with this new file; you can test this by running the command in docs/examples/README, even after importing arc. As an aside, that README also needs the filepath updating after the files were moved into src/arc, as well as in the writing-a-parser.rst file.

It's helpful (IMO) to think of isaric_transformations.py as data within ARC in the same way as the JSON schemas and TOML parsers are, rather than as a python module which can be used as such. I'd suggest moving it into the data folder, as it would need to be copied/imported into any downstream packages like ISARICAnalytics at the same time as the core & long schemas, so it makes sense for them to be in in the same place.

[pipliggins]

I'm a bit confused based on this and the previous PR if a decision has been made to not mark valid data entries in the long table with VAL?

If so, there are further changes that need to be made in this schema (i.e. the attribute_status field needs to be removed from the required list), and the documentation in docs/isaric-data-schema.rst needs updating. Most importantly, some of the logic in isaric_schema.py will need to be changed if 'VAL' is not longer an expected entry.

If not, the current schema and parser generators need fixing so the 'VAL' is included again (current changes mean it's not being included as an enum option).

[sr-murthy]

@pipliggins Thanks for your review. Looks like the changes to isaric_transformations.py need to be reverted, if it is to be treated as data rather than a library, in which case I suppose the original design of a root-level schemas folder may be better.

The change to the VAL entry also needs re-examination, because I did not attempt to change any of the codes myself.

I will update the PR shortly, and will re-request a review.

[sr-murthy]

@pipliggins PR updated with a single commit that partially reverts the changes in the previous ARC improvements PR affecting schemas and units. All critical + high priority tests pass. I also have checked that the schema and parser generation example in the docs works (from the repo root), my sample session is given below:

[/path/to/ARC] $ python3 -m adtl parse ./docs/examples/example_parser.toml ./docs/examples/example_data.csv --include-transform schemas/isaric_transformations.py
[covid-study] parsing example_data.csv: 100%|█████████████████████████████████████████████| 5/5 [00:00<00:00, 67.59it/s]
[covid-study] validating core table: 5it [00:00, 56833.39it/s]
[covid-study] validating long table: 109it [00:00, 197827.41it/s]

|table       	|valid	|total	|percentage_valid|
|---------------|-------|-------|----------------|
|core          	|4	|5	|80.000000% |
|long          	|109	|109	|100.000000% |

## core

* 1: data must contain ['subjid', 'siteid', 'dataset_id', 'dataset_disease', 'demog_sex', 'demog_age_days', 'demog_country_iso3', 'pres_adm', 'pres_date', 'outco_outcome', 'outco_date'] properties

Some minor changes in the schema and units modules, and also the relevant unit tests for these, relating to data path handling remain: mainly, the use of importlib.resources.files to locate the root path relative to the ARC package path, and renaming units/utils.py to the more meaningful units/units.py.

So the ARC Python package is basically just the ARC subpackage copied over from BRIDGE (with unit tests copied over as well).

I would appreciate a brief review again from you.

P. S. The Makefile added in the previous PR provides some useful shortcuts for running tests:

make test MARKER="<marker>"

e.g. for running critical or high tests:

make test MARKER="critical or high"

We're using the same target in the test worflows.