pvl.new not quite working

rbeyer commented 3 years ago

The intended usage for the pvl.new module was for folks to be able to easily

import pvl.new as pvl

To test out without having to mess with the body of their code. Unless they were doing something specialized, this should "just work."

However, I ran into a few problems:

If you didn't have the multidict package installed, things would fail in a very subtle, and not entirely obvious way, unless you knew what you were looking for.
In order to keep new.py as small as possible, I didn't originally override the dump() and dumps() functions, but in order for things to "just work" I needed to.
Finally, in the course of discovering all of this, I realized that while the main encoder.PVLEncoder() had group_class and object_class arguments that could be passed to let the encoder know which objects counted as PVL Groups and PVL Objects (for purposes of the rules of writing them out), all of the descendent encoders: ODLEncoder, PDS3Encoder, and ISISEncoder, didn't have those, which again caused problems.

Your Environment (please complete the following information):

OS: macOS
Environment information: Python 3.6
pvl Version 1.2.0

Additional context Working on a PR now.

michaelaye commented 3 years ago

Did I miss a message somewhere? I have no memory on what .new is all about? What is the new part doing?

rbeyer commented 3 years ago

It was a year ago, but the relevant discussion is in #52 followed by #67.

I haven't done much with it since, but the existing PVLModule classes can't be pickled because their implementation doesn't support it, but the new ones based on multidict can, which I needed for some experiments with multiprocessing, which is how I ran into the issues.

But also shows you how little this "feature" is being used. Admittedly, it isn't really documented anywhere, so there's that. I'll try and fix that in my PR, too. Doesn't help if I'm the only one who knows about it.

planetarypy / pvl

pvl.new not quite working #89