py-backwards icon indicating copy to clipboard operation
py-backwards copied to clipboard

Error triggering on this function but why?

Open Xparx opened this issue 5 years ago • 3 comments

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 avatar Sep 03 '18 19:09 Xparx

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

Xparx avatar Sep 03 '18 20:09 Xparx

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 avatar Sep 03 '18 21:09 slinnarsson

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.

slinnarsson avatar Sep 03 '18 21:09 slinnarsson