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

Error triggering on this function but why? #48

Open
Xparx opened this issue Sep 3, 2018 · 3 comments
Open

Error triggering on this function but why? #48

Xparx opened this issue Sep 3, 2018 · 3 comments

Comments

@Xparx
Copy link

Xparx commented Sep 3, 2018

The following code can't be converted from 3.6 to python 3.5 and the error seems to stem from something in py-backwards, I just can't figure out what.
I tried it in the online demo and it blew up.

def validate_spec(self, file: h5py.File) -> bool:
        """
        Validate the LoomConnection object against the format specification.

        Args:
                file:                   h5py File object

        Returns:
                True if the file conforms to the specs, else False

        Remarks:
                Upon return, the instance attributes 'self.errors' and 'self.warnings' contain
                lists of errors and warnings, and the 'self.summary' attribute contains a summary
                of the file contents.
        """
        matrix_types = ["float16", "float32", "float64", "int8", "int16", "int32", "int64", "uint8", "uint16", "uint32", "uint64"]
        vertex_types = ["int8", "int16", "int32", "int64", "uint8", "uint16", "uint32", "uint64"]
        weight_types = ["float16", "float32", "float64"]

        def delay_print(text: str) -> None:
                self.summary.append(text)

        def dt(t: str) -> str:
                if str(t).startswith("|S"):
                        return f"string"
                return str(t)

        width_ra = max([len(x) for x in (file["row_attrs"].keys())])
        width_ca = max([len(x) for x in (file["col_attrs"].keys())])
        width_globals = max([len(x) for x in file.attrs.keys()])
        width_layers = 0
        if "layers" in file and len(file["layers"]) > 0:
                width_layers = max([len(x) for x in file["layers"].keys()])
        width_layers = max(width_layers, len("Main matrix"))
        width = max(width_ca, width_ra, width_globals)

        delay_print("Global attributes:")
        for key, value in file.attrs.items():
                if type(value) is str:
                        self.warnings.append(f"Global attribute '{key}' has dtype string, which will be deprecated in future Loom versions")
                        delay_print(f"{key: >{width}} string")
                else:
                        delay_print(f"{key: >{width}} {dt(file.attrs[key].dtype)}")

        return len(self.errors) == 0

If I remove the following code it works,

delay_print("Global attributes:")
for key, value in file.attrs.items():
    if type(value) is str:
        self.warnings.append(f"Global attribute '{key}' has dtype string, which will be deprecated in future Loom versions")
        delay_print(f"{key: >{width}} string")
    else:
        delay_print(f"{key: >{width}} {dt(file.attrs[key].dtype)}")

bit this peace breaks something. Without triggering a syntax error.
The code comes from this project and file: https://github.com/linnarsson-lab/loompy/blob/master/loompy/loom_validator.py

What is going on here?

@Xparx
Copy link
Author

Xparx commented Sep 3, 2018

To simplify this somewhat.
The error seems to trigger on a type of line formated as this:
f"{key: >{width}} string"

@slinnarsson
Copy link

slinnarsson commented Sep 3, 2018

I think the issue is that py-backwards converts

f"Hello {key: >10} {x: >5} world"

into

''.join(['Hello ', '{: >10}'.format(key), ' ', '{: >5}'.format(x), ' world'])

So, basically treating each { } fragment separately, and then joining together all the pieces. With a naïve search for pairs of curly braces, I can see how this will fail when curly braces are nested.

A possible solution would be to convert instead to:

"Hello {key: >10} {x: >5} world".format(**{**locals(), **globals()})

which simply requires finding all f-strings, removing the f, and appending an expression that is always the same. I think that's likely to always work, but maybe there's a performance hit (though I suspect the actual string formatting is slower than merging two dictionaries).

I tested the specific case above, and the following two expressions both work and give equivalent results:

f"{key: >{width}} string"  # Python 3.6
"{key: >{width}} string".format(**{**locals(), **globals()})  # Converted to Python 3.5

@slinnarsson
Copy link

Oh, I realize you need to put the actual expressions in the arguments (since f-strings allow expressions inside the curly braces). So it would have to be something like:

f"{key: >{width + 2}} string"  # Python 3.6
"{a: >{b}} string".format(a=key, b=width + 2)  # Converted to Python 3.5

which means you'll need to parse the nested curly braces anyway.

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

2 participants