Skip to content

Improve DeprecationWarning #109

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

Open
michaelaye opened this issue Mar 13, 2025 · 5 comments
Open

Improve DeprecationWarning #109

michaelaye opened this issue Mar 13, 2025 · 5 comments

Comments

@michaelaye
Copy link
Member

Using version 1.3.2 via conda python 3.12

I'm getting:

PendingDeprecationWarning: The pvl.collections.Units object is deprecated, and may be removed at the next major patch. Please use pvl.collections.Quantity instead.

even so I am not using the mentioned objects.
It seems, that when pvl.load() is called to parse a PDS label file, it internally creates Units objects if there are units in the PDS label. This is happening even though my code isn't directly using the Units class, hence the confusion.

Could this be solved differently somehow, maybe inform the user that it's an internal deprecation, or how to exactly use the Quantity object, even so I'm not controlling it?

Thanks!

@rbeyer
Copy link
Member

rbeyer commented Mar 13, 2025

Interesting. If that's happening internally, then internally, we should switch that to use the "proper" Quantity unit so that the library itself isn't using things that are deprecated. Agreed, if the user isn't using the Units object, they shouldn't be seeing the warning. Now sure when I'll get to this, but should be straightforward to solve.

@michaelaye
Copy link
Member Author

possibly your thought was, that even so it's used internally, it might surprise the user by what they find in the resulting object, if you would just change that yourself.

@rbeyer
Copy link
Member

rbeyer commented Mar 15, 2025

I must be getting rusty. I can't replicate the problem you're having. If I pvl.load() a PDS3 label file with units, I don't see that warning:

>>> import pvl
>>> foo = pvl.load("units1.lbl")
>>> print(foo)
PVLModule([
  ('PDS_VERSION_ID', 'PDS3')
  ('MSL:COMMENT', 'THING TEST')
  ('FLOAT_UNIT', Quantity(value=0.414, units='KM'))
  ('INT_UNIT', Quantity(value=4, units='M'))
])
>>>

The units1.lbl file above is in the pvl test data here: https://github.com/planetarypy/pvl/blob/main/tests/data/pds3/units1.lbl

I also looked through the library, it isn't instantiating the old pvl Units() object anywhere (except in some tests), so I'm not sure why you're seeing that behavior. The default behavior in the decoder classes is to default to using the new Quantity() class if there is PVL-text encountered with units (as you can see in the example above), so I just don't know what would be producing this warning.

Can you provide a minimal working example that demonstrates the emission of the warning?

@mcbeth
Copy link

mcbeth commented Apr 2, 2025

python -Werror -c "import pvl"

triggers the warning for me on 1.3.2

@rbeyer
Copy link
Member

rbeyer commented Apr 3, 2025

Aha! I had put the PendingDeprecationWarning in the wrong place. Its now fixed in PR #110

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants