TypeError in _get_potcar_symbols()

[janosh]

class Lobsterin()._get_potcar_symbols() causes CI errors in atomate

pymatgen/pymatgen/io/lobster/inputs.py

Lines 612 to 633 in fd553a8

	potcar = Potcar.from_file(POTCAR_input)
	for pot in potcar:
	if pot.potential_type != "PAW":
	raise OSError("Lobster only works with PAW! Use different POTCARs")
	# Warning about a bug in lobster-4.1.0
	with zopen(POTCAR_input, "r") as f:
	data = f.read()
	if "SHA256" in data or "COPYR" in data:
	warnings.warn(
	"These POTCARs are not compatible with "
	"Lobster up to version 4.1.0."
	"\n The keywords SHA256 and COPYR "
	"cannot be handled by Lobster"
	" \n and will lead to wrong results."
	)
	if potcar.functional != "PBE":
	raise OSError("We only have BASIS options for PBE so far")
	Potcar_names = [name["symbol"] for name in potcar.spec]
	return Potcar_names

        with zopen(POTCAR_input, "r") as f:
            data = f.read()
>       if "SHA256" in data or "COPYR" in data:
E       TypeError: a bytes-like object is required, not 'str'

Triggered by PR: hackingmaterials/atomate#738

[JaGeo]

Will have a look at it as soon as I am back from my vacation ... (On Monday)

It's just there to warn users if they use the POTCARs including these keywords. If you know how to fix it, please go ahead. Errors seem to be new, though...

And, thanks for putting all the work into this. 🙂

[janosh]

I guess one way to fix it would be

- if "SHA256" in data or "COPYR" in data: 
+ if b"SHA256" in data or b"COPYR" in data: 

but I was a bit confused by this error as I didn't expect the result f.read() to be a bytes object in the first place. Since data is prob supposed to be str, turning "SHA256" into bytes string wouldn't be the right fix here.

[JaGeo]

Yeah, same. I hope it is not again some test data problem in atomate. 🙈

[janosh]

Don't think so. Since zopen is coming from monty maybe @shyuep can comment how to handle this.

[JaGeo]

At least in pymatgen, the tests seem to be working:

pymatgen/pymatgen/io/lobster/tests/test_lobster.py

Line 2012 in fd553a8

def test_get_potcar_symbols(self):

[shyuep]

zopen does not do anything to the encoding. It merely passes through to io.open or gzip or bzip2 based on the file extension. So this is not related to zopen. In general, python has become more strict on binary vs text types. So you might have to properly handle the encoding conversions.

[janosh]

@JaGeo Thanks, yes I saw the tests pass here. The difference is prob that in atomate the POTCAR is gzipped so the returned IO stream comes from gzip.open() instead of python's open().

@shyuep You mean check the type and handle the comparison accordingly?

if isinstance(data, bytes):
    if b"SHA256" in data or b"COPYR" in data:
        # ...
else:
    if "SHA256" in data or "COPYR" in data: 

[shyuep]

No, you should handle the encoding of "data" first. If it is supposed to be a string, it needs to be converted to unicode first. The comparison should be based on unicode, not bytes. Then everything that follows using data should treat it as a proper unicode.