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

Load_func module #11

Merged
merged 3 commits into from
Dec 24, 2023
Merged

Load_func module #11

merged 3 commits into from
Dec 24, 2023

Conversation

MariaGorodetski
Copy link
Contributor

I have added a new module, which is supposed to contain all future 'show_fundus' functions, now called 'load_image'.
Now, you don't need to specify the specific function you want to use in the code (though you still can, for now). Instead, you need to add your field_type and the corresponding function it should use in the dictionary properties sheet, which will be updated in the general metadata directory.

@alondmnt
Copy link
Contributor

nice! will have a look

Copy link
Contributor

@alondmnt alondmnt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this a lot. Good job!

Returns:
str: the path to the file
"""
path = os.path.join(DATASETS_PATH, 'metadata', '2 - Dictionary properties - field_type.csv')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not my favorite file name, but I won't insist.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, but this is consistent with the original file
I hope this is temporary until we can store it in a different location, such as Athena or another type of storage service

Comment on lines 23 to 28
function_name = FIELD_TYPE_TO_FUNC.get(field_type)
if function_name is not None:
return globals().get(function_name)
else:
# Handle the case where the field type is not found
return pd.read_parquet
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what should we do if the function name is not in the globals? it will return a None, and there will be a downstream error, but perhaps we should catch it here first ("field type load func is missing..." of sorts)


minor style suggestion (unless you object), instead of the if-else clause you may use:

function_name = FIELD_TYPE_TO_FUNC.get(field_type, 'read_parquet')

and have from pandas import read_parquet at the top of the py.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! added to the new commit
Please review :)

Copy link
Contributor

@alondmnt alondmnt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, thanks! merge away

@MariaGorodetski MariaGorodetski merged commit d00e267 into dev_v12 Dec 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants