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

Disambiguate the routine name initialize_inputs in picmi.py #4554

Conversation

dpgrote
Copy link
Member

@dpgrote dpgrote commented Dec 20, 2023

This PR improves code readability in the picmi interface. Previously, each class had a method called initialize_inputs, but with all classes having a routine of the same name, it made it hard to find specific versions of the method. This PR adds prefixes to the name, based on the input type of the class. For instance, in the diagnostic classes, the routine is called diagnostic_initialize_inputs.

@dpgrote dpgrote added the component: PICMI Standardized input format label Dec 20, 2023
@ax3l ax3l added the cleaning Clean code, improve readability label Dec 21, 2023
@roelof-groenewald
Copy link
Member

@ax3l had the idea that instead of prefixing the function name by the class type, the argument types for each individual initialize_inputs function could be specified (see https://docs.python.org/3/library/typing.html) which would similarly allow for the identification of the function definition.
That might be a cleaner solution, what do you think @dpgrote?

@dpgrote
Copy link
Member Author

dpgrote commented Dec 21, 2023

@ax3l had the idea that instead of prefixing the function name by the class type, the argument types for each individual initialize_inputs function could be specified (see https://docs.python.org/3/library/typing.html) which would similarly allow for the identification of the function definition. That might be a cleaner solution, what do you think @dpgrote?

@roelof-groenewald Unfortunately, that doesn't solve the issue that when editing the file, I have to search around to find the right routine I'm looking for, since I have to scan over all of the routines of the same name. Also, some of the routines from different categories don't have any arguments.

Copy link
Member

@roelof-groenewald roelof-groenewald left a comment

Choose a reason for hiding this comment

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

As discussed we will merge this now and in a follow-up PR (at some later time) add type hints as well.

@roelof-groenewald roelof-groenewald merged commit a3e04a7 into ECP-WarpX:development Jan 16, 2024
39 checks passed
@dpgrote dpgrote deleted the picmi_disambiguate_initialize_inputs branch January 17, 2024 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleaning Clean code, improve readability component: PICMI Standardized input format
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants