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

Why get_function_source_code return source file of the function not the source function itself? #4543

Closed
unkcpz opened this issue Nov 5, 2020 · 3 comments · Fixed by #4554

Comments

@unkcpz
Copy link
Member

unkcpz commented Nov 5, 2020

def get_function_source_code(self):
"""Return the absolute path to the source file in the repository.
:returns: the absolute path of the source file in the repository, or None if it does not exist
"""
return self.get_object_content(self.FUNCTION_SOURCE_FILE_PATH)

Would this more make sense if the source code of function rather than the source of the file is returned?
Besides, if I understand it correctly, every process function will be stored along with a source file in the repository. Is that too redundant to store the same files every time when the ProcessNode call and store?

@sphuber
Copy link
Contributor

sphuber commented Nov 5, 2020

Not sure, because the rest of the file can also be important for the process function, for example other functions that may be defined or imports. Of course we can add a method that gives just the source code of the function itself, and potentially rename the current one, that would be possible I would say.

I just noticed by the way that the docstring is incorrect, the :returns: is incorrect as it says the filepath will be returned.

With respect to the duplication of the source code: this is indeed very inefficient, but fortunately with the new repository implementation that will be merged for aiida-core==2.0.0, that will be automatically fixed. That repo implementation will automatically deduplicate content.

@unkcpz
Copy link
Member Author

unkcpz commented Nov 6, 2020

I just find that display the source code of the function rather than the whole file is much concise in Material Cloud explore. I propose to add the method to give the source code.
However, It might not be a good idea to store the source of the function as an attribute.

@ltalirz
Copy link
Member

ltalirz commented Nov 11, 2020

I propose to add the method to give the source code.

That sounds good to me.

However, It might not be a good idea to store the source of the function as an attribute.

I agree that this can be problematic and would advise against it, see also #3714
The basic principle should be to store in the DB what you want to query for and I imagine that querying the source code of workfunctions is not really a use case people have (?)

@sphuber sphuber added this to the v2.1 milestone Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants