Continued need for use of tostring() in codebase?

[matthewfeickert]

In the pyhf test logs for tests/test_import.py we're seeing a lot of (harmless) warnings of

/opt/hostedtoolcache/Python/3.8.3/x64/lib/python3.8/site-packages/uproot/source/cursor.py:73: DeprecationWarning: tostring() is deprecated. Use tobytes() instead.
return source.data(start, stop).tostring()

where the offending function is

https://github.com/scikit-hep/uproot/blob/634667fad826ec6c86e2df442887b1024c2cfee8/uproot/source/cursor.py#L63-L73

Is there an explicit need for this still in the codebase? If no, would a PR to switch it over be of interest, or should any contributions be left for uproot4?

cc @lukasheinrich @kratsg

[kratsg]

Should we just wait on a switch to uproot4 perhaps?

[jpivarski]

Actually, this is a wake-up call to me that ndarray.tostring is deprecated—I thought I was using a recent version (conda-forge), but I haven't been seeing these warnings. I'll be sure to fix it in Uproot 4. (Its name is clearly a relic of Python 2.)

@kratsg, you're right that that's where development should focus, but I also didn't know that this was an issue, and can not take care of it. Fortunately, these calls are more centralized now because Uproot 4 converts to strings early.

In the other hand, it's also simple enough that it can be fixed in both versions. I'll be using a hasattr to determine whether the version of ndarray had a tobytes attribute before calling it because some allowed versions of NumPy do not. (I'll have to check mine, which is running under Python 3.7, I think.)

[matthewfeickert]

Fortunately, these calls are more centralized now because Uproot 4 converts to strings early.

This all sounds good. :+1:

In the other hand, it's also simple enough that it can be fixed in both versions.

If you decide to go this route and would like help on anything, let us know.

[jpivarski]

I've verified that this is done in all cases. (There's a switch for tobytes vs tostring to support old NumPys, but it happens in a centralized place, so it doesn't add much complexity.)

[matthewfeickert]

I've verified that this is done in all cases.

@jpivarski, just to clarify this is now done in uproot4, yes?

[jpivarski]

That is correct. I closed it because it's done in the new codebase. (The old codebase can have things like this because it's about maintaining functionality that already works for users—it might prevent them from updating to the latest NumPy with the old Uproot, but if they can update NumPy, they can update Uproot.)