Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dose_unit_conversions missing in eicupreparator.py #32

Closed
kanghyunyu opened this issue Jul 3, 2024 · 4 comments
Closed

dose_unit_conversions missing in eicupreparator.py #32

kanghyunyu opened this issue Jul 3, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@kanghyunyu
Copy link

Unlike other datasets, eICU seems to be missing the variable "dose_unit_conversions" in "eicupreparator.py".

Is this meant to be? It ends up raising "ValueError: not enough values to unpack (expected 2, got 0)" in "newmedicationprocessor.py", as the return value for "self.dose_unit_expressions, self.dose_unit_replacements = self._dose_unit_expressions()" is an empty list.

@USM-CHU-FGuyon USM-CHU-FGuyon added the bug Something isn't working label Jul 3, 2024
@USM-CHU-FGuyon
Copy link
Owner

eICU has hundreds of units for medication dosage, which is why I did not provide the dose_unit_conversion dictionnary nor support for drug dosage in eICU.

However it should not raise an Error, I'm fixing this as soon as I can.

As an instant fix, replacing

        exprs = []
        if self.dose_unit_conversion_dic is None:
            return exprs

by

        exprs = []
        if self.dose_unit_conversion_dic is None:
            return exprs, {}

Should be fine.

Please tell me if this fixes the issue, I can't run the codes at the moment.

@kanghyunyu
Copy link
Author

I see. This works (although sink_parquet fails for medication and gets diverted to pl.writing. Hope that's normal).

@USM-CHU-FGuyon
Copy link
Owner

For medications it should not be too problematic to do pl.write_parquet because tables size remain reasonable.

I was a lot more focused on enabling sinking for larger tables to avoid memory error.

thanks for contributing, I'll push the change asap
Matthieu

@kanghyunyu
Copy link
Author

Great, thanks for all the work!

USM-CHU-FGuyon added a commit that referenced this issue Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants