search

cdisc-org / cdisc-rules-engine

Open source offering of the cdisc rules engine
MIT License
45 stars 12 forks source link

add new argument to get_variables_metadata calls in dataset builders #727

Closed nhaydel closed 3 weeks ago

nhaydel commented 3 weeks ago

Recently a new argument was added to the get_variables_metadata method of the LocalDataService, the dataset builders did not provide this argument. This PR adds the new argument to get_variables_metadata function calls.

Steps to test:

Verify rule CORE-000334 runs correctly on an xpt file

SFJohnson24 commented 3 weeks ago

I am not thrilled about how much we use dataset_name to mean dataset_path throughout the codebase but that convention seems to be fairly widespread. the code runs as expected for CG0016 with builders arguments fix and previous_operations implementation passing review and is ready for merge.

nhaydel commented 3 weeks ago

I am not thrilled about how much we use dataset_name to mean dataset_path throughout the codebase but that convention seems to be fairly widespread. the code runs as expected for CG0016 with builders arguments fix and previous_operations implementation passing review and is ready for merge.

I agree, this naming issue is due to the fact that the engine used to be a cloud based solution and each dataset had a "name" or "id" instead of a path. I think it's worth renaming these variables, but like you mentioned it would take time