1421: load variables ccode correctly

[alexfurmenkov]

No description provided.

[RamilCDISC]

Could you please add details to the PR explaining why this check was removed and summarize your overall work?

[alexfurmenkov]

After I fixed the dataset builder to correctly load library_variable_ccode the rule still wasn't working as expected, the equality checks just didn't work.

I've started diving in and noticed that after calling add_comparator_to_rule_conditions on this rule it becomes corrupt:

Pay attention to the conditions 2 and 3 - comparator values become define_library_variable_ccode whereas they should be library_variable_ccode and define_variable_ccode

[SFJohnson24]

I am getting an error when I run Marcelina's data.
On line 159

        """
        Gets Define XML variables metadata.
        """
        define_xml_reader = DefineXMLReaderFactory.get_define_xml_reader(
            self.dataset_path, self.define_xml_path, self.data_service, self.cache
        )
        return define_xml_reader.extract_variables_metadata(
            domain_name=self.dataset_metadata.domain
        )

extract_variables_metadata is being called with the None for domain for SUPPEC. We need a check for self.dataset_metadata.is_supp() and if so call extract_variables_metadata with the rdomain property. You could also do a check if domain exists, and if not, use rdomain.

the other issue I see would occur downstream from this: _get_domain_metadata() in base_define_xml__reader is looking only based on domain and grabbing the first that matches. This will be an issue when differentiating Supp-- from Parents.

I suspect passing the name and the domain/rdomain may be necessary and using the Name and the Domain property of the itemgroupdef to pick which metadata to grab would be best plan of attack

[SFJohnson24]

see comment

[SFJohnson24]

this is the problem I described in my comment above. "the other issue I see would occur downstream from this: _get_domain_metadata() in base_define_xml__reader is looking only based on domain and grabbing the first that matches. This will be an issue when differentiating Supp-- from Parents."

we need to update extract_variables_metadata to not only use domain but also name as if we use the rdomain--it will grab the parent as they both have domain set to the same thing. There is a name property 'SUPP--' that will be the same as self.dataset_metadata.name. This can be used in the case of supps to correctly retreive the supp data and not the parent dataset metadata

[SFJohnson24]

see comment

[SFJohnson24]

This PR correctly resolves the issues with fetching library ccodes for dataset builder datasets.


this show the ccode populated Marcelina found which is now correctly populated. It also resolves the above comment where we use the name and the domain to find the supp-- datasets instead of grabbing the domain (suppec using just domain would grab the ec dataset and not the suppec data)

[RamilCDISC]

Validation is clear from QA. The ticket has also been validated by @SFJohnson24 .