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

fypp with pypy throwing a FyppFatalError/NameError #32

Open
MuellerSeb opened this issue Aug 28, 2023 · 7 comments
Open

fypp with pypy throwing a FyppFatalError/NameError #32

MuellerSeb opened this issue Aug 28, 2023 · 7 comments

Comments

@MuellerSeb
Copy link

When using fypp with pypy, I get the following strange error:

common.fypp:66: error: exception occurred when setting variable(s) 'VERSION90' to 'defined('VERSION90')' [FyppFatalError]
  error: name 'defined' is not defined [NameError]

(file from Fortran stdlib)

Don't know what goes wrong here...

@MuellerSeb
Copy link
Author

Some more context:

This happens on a conda-forge feedstock but only for aarch64 and ppc64le platforms with this docker: quay.io/condaforge/linux-anvil-cos7-x86_64

See here for reference: conda-forge/mhm-feedstock#15

@aradi
Copy link
Owner

aradi commented Aug 28, 2023

Thanks, it seems I can reproduce it, after installing pypy with Conda on x86/Linux, I get the same error message. I have not the slightest idea, why this happens, I'd have to check the details...

@zerothi
Copy link

zerothi commented Nov 19, 2024

I think the smallest reproducer is:

program t
print *, ${NAME}$
end program

Trying to fyppifying this with:

$> fypp -DNAME=hello test.fypp
error: exception at evaluating 'hello' in definition for 'NAME' [FyppFatalError]
error: name 'hello' is not defined [NameError]

The problem is that fypp tries to resolve the name (if it ain't stringified).
This is a bit problematic when one, e.g. wishes to use iso_fortran_env names for variables:
-DINT_TYPE=int32, and a fortran code: integer(${INT_TYPE}$) (this won't work!).
I can see why this is hard to figure out, is the name a variable, or a string?

The solution is something obscure like this:

$> fypp -DNAME="'hello'" test.fypp

to fully escape the name as a string (either single string will be shell-resolved and not passed to fypp).

I don't know of an elegant solution other than automatically stringifying anything it doesn't name-resolve, but even then it is problematic because you don't know whether it should be a name, or whether it really should be a string.

@aradi
Copy link
Owner

aradi commented Nov 19, 2024

Yes, Fypp currently evaluates everything after the = sign as a Python expression. So, yes, you have to pass the string-delimiters either with -DNAME="'hello'" or with -DNAME=\"hello\". We could easily introduce a second kind of define (like in CMake), so that the right hand side is automatically interpreted as a string -SNAME=hello and one could even enforce conversion (if needed) with the explicit naming of the conversion function, like -SNAME:bool=True.

Would that be something which would make your life easier?

@zerothi
Copy link

zerothi commented Nov 19, 2024

Yes, Fypp currently evaluates everything after the = sign as a Python expression. So, yes, you have to pass the string-delimiters either with -DNAME="'hello'" or with -DNAME=\"hello\". We could easily introduce a second kind of define (like in CMake), so that the right hand side is automatically interpreted as a string -SNAME=hello and one could even enforce conversion (if needed) with the explicit naming of the conversion function, like -SNAME:bool=True.

Would that be something which would make your life easier?

Oh, yes, I actually thought of this, thinking I shouldn't propose it !!! ;) haha...

I think this could be nice, in particular the delimiters required for string handling in cmake projects are annoying, but not only that, it is also error prone, say if the user them-selves put in quotation that gets caught by cmake, what should one do then?

My thought on initial variable names would be:

  • :INT
  • :FLOAT
  • :PATH (necessary?)
  • :STRING (or just :STR)
  • :BOOL
  • :NONE for a define only variable? Or is it overkill? Sometimes it can be hard to know whether a definition actually has a meaning somewhere...
  • :ARRAY(<any of the above>[, delim=,]), say -DH:ARRAY(str,;)=int32;int64 or -DH:ARRAY(str)=int32,int64 or similar?

Might have omitted some? Or are some just overkill?

@aradi
Copy link
Owner

aradi commented Nov 19, 2024

That's definitely an overkill, and brought me to change my mind about the converters. 😆

My suggestion would be to implement the -S option first without any conversion possibility. This would already allow to remedy the problems with the tricky quotation when passing strings. For all other the cases (e.g. where a conversion is needed), one can use the good old -D option with any valid Python expression.

As an example, given the file test.fypp with

$:array

The command

fypp -m numpy -Darray='numpy.array([1, 2, 3], numpy.float32)' test.fypp

results in

[1. 2. 3.]

Note, the quotation in the command above is only needed to prevent bash to evaluate the square brackets...
So, probably, we should not try to add complicated converters for -S, if the same functionality can be easily used with the -D option already. Let's just fix the only issue, which is more complicated then it needs to be: the passing of string values when defining entites on the command line.

@zerothi
Copy link

zerothi commented Nov 20, 2024

Agreed, let's keep it simple, the fypp -SVAR=hello seems like a nice and simple addition!

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

No branches or pull requests

3 participants