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

Consistently map function names to their "canonical" names #68

Closed
ranocha opened this issue Aug 18, 2020 · 6 comments · Fixed by #200
Closed

Consistently map function names to their "canonical" names #68

ranocha opened this issue Aug 18, 2020 · 6 comments · Fixed by #200
Labels
discussion enhancement New feature or request

Comments

@ranocha
Copy link
Member

ranocha commented Aug 18, 2020

In GitLab by @sloede on May 28, 2020, 08:35

As a follow up to a discussion with @ranocha, I think we should use a consistent way to map function names to their canonical names for documentation/parameter file purposes.

Example

The function initial_conditions_gauss_pulse should be consistently be called either initial_conditions_gauss_pulse or just gauss_pulse when referring to it in strings. That is, we want to either use

initial_conditions = "initial_conditions_gauss_pulse"

and have

julia> get_name(initial_conditions_gauss_pulse)
"initial_conditions_gauss_pulse"

or

initial_conditions = "gauss_pulse"

and have

julia> get_name(initial_conditions_gauss_pulse)
"gauss_pulse"

Since for me, the initial_conditions_ prefix is actual not part of the function name but is used to put it in a namespace, I prefer the second variant, as context alone should be sufficient to determine what kind of function it is. It reduces typing and feels more natural to me, similarly, I wouldn't say "I am Human Michael" but just "I am Michael" (unless I need to emphasize that I am not your machine overlord).

@ranocha ranocha added discussion enhancement New feature or request labels Aug 18, 2020
@ranocha
Copy link
Member Author

ranocha commented Aug 18, 2020

In GitLab by @sloede on May 28, 2020, 08:36

mentioned in merge request !57

@ranocha
Copy link
Member Author

ranocha commented Aug 18, 2020

In GitLab by @ranocha on May 28, 2020, 08:49

I prefer the second variant, as context alone should be sufficient to determine what kind of function it is.

We can do that. The required changes seem to be

  1. Specialize get_names accordingly.
  2. Add additional logic to the parsing part, e.g.
    julia> fluxname = "chandrashekar"
    "chandrashekar"
    
    julia> try 
               flux = eval(Symbol(fluxname))
           catch e
               flux = eval(Symbol(fluxname * "_flux"))
           end

@ranocha
Copy link
Member Author

ranocha commented Aug 18, 2020

In GitLab by @sloede on May 28, 2020, 08:55

Ah, I forgot that my approach would also require external users to always use a _flux subscript for their new flux functions, which is of course very inconvenient and impractical. OTOH, allowing both forms in a parameter file, e.g., central and central_flux is also suboptimal - there should always be just a single name.

Maybe we just use the long names for now and bench this discussion until we have settled on an approach for how to handle parameters in the future?

@ranocha
Copy link
Member Author

ranocha commented Aug 18, 2020

In GitLab by @sloede on May 28, 2020, 09:43

changed title from Consistently map function names to {-its-} "canonical" names to Consistently map function names to {+their+} "canonical" names

@ranocha
Copy link
Member Author

ranocha commented Aug 18, 2020

In GitLab by @sloede on May 28, 2020, 10:57

changed the description

@ranocha
Copy link
Member Author

ranocha commented Aug 18, 2020

In GitLab by @gregorgassner on May 28, 2020, 11:28

In my personal opinion, the current version as in the merge request is fine for me! I do not mind to type initial_condition one more time.

Imo, it makes it clear to read and that is good in my book.

@ranocha ranocha mentioned this issue Nov 9, 2020
45 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant