backwards compatibility ridge_transforms

[michaelchin]

          @michaelchin - tests are failing now. To help with backwards compatibility you could retain the `ridge_transforms` attribute which is the combination of BOTH ridges and transform boundaries.

Originally posted by @brmather in #243 (comment)

[brmather]

There are also many examples of gplot.plot_ridges_and_transforms which no longer work in the Notebooks.

Rather than remove all of these instances, I suggest you simply reuse the ridge_transforms attribute. Just combine all the ridges under self.ridges and all the transforms under self.transforms into self.ridge_transforms.

[michaelchin]

I can bring back self.ridge_transforms. But it will not be the same as before. Now the self.ridges contains all gpml:MidOceanRidge, which is the same as the old self.ridge_transforms. The new self.transforms contains all gpml:Transforms, which is the old self.misc_transforms.

If I bring back self.ridge_transforms, the notebooks will work, but produce different results quietly. Not sure if we should break customers' code intentionally to alert customers the change. People might be more angry if they found the results had changed quietly( because errors are more difficult to notice/understand).

Maybe set self.ridge_transforms = self.ridges??? But it will not include all gpml:Transforms. NW might not happy...

So, I guess the customers will be angry for sure. But we need to think carefully how to make them less angry. Their code need to change anyway because we did not separate the ridges and transforms properly before. I think we should let the customers know the problem. Breaking their code may be the easy way to get their attention.

Alternatively, we can print warning messages. But people tend to ignore warnings...

[michaelchin]

@jcannon-gplates @nickywright

This is a sequel of #227.

The next release will be 2.0 major release. If we want to get rid of self.ridge_transforms eventually, this is a good chance. People will be more understanding if their code breaks due to a major release change, just like numpy 2 is breaking everything.

What do you think?

[brmather]

I can bring back self.ridge_transforms. But it will not be the same as before. Now the self.ridges contains all gpml:MidOceanRidge, which is the same as the old self.ridge_transforms. The new self.transforms contains all gpml:Transforms, which is the old self.misc_transforms.

So I originally intended that self.ridge_transforms was to be what it would be now (i.e. gpml ridges and transforms). As a compromise maybe you can remove the self.ridge_transforms attribute, but keep the plot_ridges_and_transforms function which simply calls the plot_ridges and plot_transforms functions. You could also add a warning. People love warnings.

The next release will be 2.0 major release. If we want to get rid of self.ridge_transforms eventually, this is a good chance. People will be more understanding if their code breaks due to a major release change, just like numpy 2 is breaking everything.

I generally agree that 2.0 is a good opportunity to break things. But let's also try not create more work for ourselves.

[nickywright]

I'm confused, what is self.ridge_transforms doing exactly? Plotting specifically both the gpml.MidOceanRidge and gpml.Transform type at once, or just plotting the full output of gpml.MidOceanRidge (no splitting of the gpml.MidOceanRidge into ridge and transform types).

[jcannon-gplates]

I'm confused, what is self.ridge_transforms doing exactly? Plotting specifically both the gpml.MidOceanRidge and gpml.Transform type at once, or just plotting the full output of gpml.MidOceanRidge (no splitting of the gpml.MidOceanRidge into ridge and transform types).

When you say "both the gpml.MidOceanRidge and gpml.Transform type" does that include the "full output" of gpml.MidOceanRidge ? My understanding is plotting gpml:MidOceanRidge means plotting the full geometry of that feature type (ie, not split into ridges/transforms). And plotting gpml:Transform means plotting features of that type (ie, does not include transforms split out from gpml:MidOceanRidge). So I think whenever we mention a feature type it means plotting its full geometry (no splitting). And I agree with you, if we later add back ridge/transform separation then we should call it something completely different (something new).

So I think Ben originally intended (if I understand correctly; please correct if I'm wrong) for self.ridge_transforms to plot both gpml.MidOceanRidge and gpml.Transform. But somehow it ended up plotting only gpml.MidOceanRidge - probably because self.ridge_transforms was (mis-)understood to mean the ridge and transform segments of gpml.MidOceanRidge (and hence excluded gpml.Transform). And then I guess self.misc_transforms was later added to cover the excluded gpml.Transform.

So I think Ben is now suggesting that self.ridge_transforms (or possibly just plot_ridges_and_transforms) should satisfy that original intention (ie, gpml.MidOceanRidge and gpml.Transform).

[brmather]

Bang on, John!

[jcannon-gplates]

In terms of backward compatibility, yeah unfortunately it seems unavoidable for the most part.

I guess there's two paths (or a combination of the two):

1. Remove all the old functions/attributes and come up with totally new function/attribute names to avoid surprising users with changes under the hood (as Michael mentioned). In other words, we force users code to break so that they'll have to update it.
2. Re-use some function/attribute names - just because they're so descriptive (like ridges and transforms). Although, you could argue that it's confusing, right. Does ridges mean gpml:MidOceanRidge, or just the ridge sections from them. Similar with transforms.

Since the confusion as to what-is-a-ridge and what-is-a-transform is why we're changing all this, perhaps it's OK to re-use some function/attribute names for the sake of clarity for future users. Such as self.ridges now meaning gpml:MidOceanRidge (instead of just its split-out ridge sections), and self.transforms now meaning gpml:Transform (instead of just its split-out transform sections from gpml:MidOceanRidge). Should we add warnings there too? Might be a good idea.

As a compromise maybe you can remove the self.ridge_transforms attribute, but keep the plot_ridges_and_transforms function which simply calls the plot_ridges and plot_transforms functions. You could also add a warning. People love warnings.

That sounds good.

Also I'd be fine with keeping self.ridge_transforms. But adding a warning when accessing it - perhaps only until version 2.1 (ie, not forever). I guess we're already keeping self.ridges and self.transforms, which also now have changed meanings (and probably deserve warnings too).