GPlates / gplately

GPlately is a Python package to interrogate tectonic plate reconstructions.
https://gplates.github.io/gplately/
GNU General Public License v2.0
60 stars 13 forks source link

backwards compatibility `ridge_transforms` #250

Open michaelchin opened 3 months ago

michaelchin commented 3 months ago

Discussed in https://github.com/GPlates/gplately/discussions/248

Originally posted by **michaelchin** August 5, 2024 @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 https://github.com/GPlates/gplately/issues/243#issuecomment-2268085970_
michaelchin commented 3 months ago
jcannon-gplates commented 3 months ago

Do we want to bring back misc_transform attribute (and related functions)?

We could probably skip it since it's now taken care of by transforms (which contains gpml:Transform).

michaelchin commented 3 months ago

Do we want to bring back misc_transform attribute (and related functions)?

We could probably skip it since it's now taken care of by transforms (which contains gpml:Transform).

I guess it has the same backwards compatibility problem

jcannon-gplates commented 3 months ago

Maybe we should just remove it, and have it hard fail - it's a bit of a confusing attribute to keep around IMO? That would mean changing misc_transforms to transforms in the notebooks, etc, though.

jcannon-gplates commented 3 months ago

Oh, you probably meant keep it around just so you can emit a warning. Yeah that makes sense. And I guess eventually it could get removed. We can probably make it a deprecation warning then.

michaelchin commented 3 months ago
  • misc_transforms attribute

Eventually both misc_transforms and ridge_transforms should be removed. There is no enough reason for them to stay and they may cause confusion. People may wonder why there is a function plot_ridges_and_transforms(). Can't I just plot_ridges(); plot_transforms()?

Anyway, just give users a grace period to change their code voluntarily. If they don't, we force them to do it in the future release.

michaelchin commented 3 months ago

Maybe misc_transforms is a bit worse than ridge_transforms. But it seems to me both of them are too bad to stay.

That would mean changing misc_transforms to transforms in the notebooks, etc, though.

I think this is the reason why Ben wants to keep ridge_transforms. But maybe misc_transforms is not used as much as ridge_transforms. Frankly I don't see how hard it is to change them.

Anyway, let's be nice and give users a grace period .

jcannon-gplates commented 3 months ago

Sounds good. Deprecation warnings for stuff that will eventually be removed. And regular warnings for stuff whose meaning is changing. Then users can ignore them until something doesn't work as they expected ;-)

michaelchin commented 3 months ago
michaelchin commented 3 months ago
michaelchin commented 3 months ago
michaelchin commented 3 months ago

add warning message for plot_ridges() "The 'plot_ridges' function has been changed since GPlately release 1.3.0. Now the 'plot_ridges' function plots all gpml:MidOceanRidge features in the reconstruction model. You need to check your workflow to make sure the new 'plot_ridges' function still suits your purpose. In the previous GPlately releases, the 'plot_ridges' function plots only the ridges in the gpml:MidOceanRidge features(the transforms in the gpml:MidOceanRidge features are not plotted)."

michaelchin commented 3 months ago

add warning message for plot_transforms() "The 'plot_transforms' function has been changed since GPlately release 1.3.0. Now the 'plot_transforms' function plots all the gpml:Transform features in the reconstruction model. You need to check your workflow to make sure the new 'plot_transforms' function still suits your purpose. In the previous GPlately releases, the 'plot_transforms' function plots only the transforms in the gpml:MidOceanRidge features(all the gpml:Transform features are not plotted)."

michaelchin commented 3 months ago
michaelchin commented 3 months ago
michaelchin commented 3 months ago

add warning message for get_ridges() "The 'get_ridges' function has been changed since GPlately release 1.3.0. Now the 'get_ridges' function returns all gpml:MidOceanRidge features in the reconstruction model. You need to check your workflow to make sure the new 'get_ridges' function still suits your purpose. In the previous GPlately releases, the 'get_ridges' function returns only the ridges in the gpml:MidOceanRidge features(the transforms in the gpml:MidOceanRidge features are not plotted)."

michaelchin commented 3 months ago

add warning message for get_transforms() "The 'get_transforms' function has been changed since GPlately release 1.3.0. Now the 'get_transforms' function returns all the gpml:Transform features in the reconstruction model(the transforms in the gpml:MidOceanRidge features are not included). You need to check your workflow to make sure the new 'get_transforms' function still suits your purpose. In the previous GPlately releases, the 'get_transforms' function returns only the transforms in the gpml:MidOceanRidge features(the gpml:Transform features are not plotted)."

michaelchin commented 3 months ago

allow users to disable "ridge and transform" related warning

michaelchin commented 2 months ago

allow users to disable "ridge and transform" related warning

No, we should annoy users so that they have the motive to update their code with the new API.

michaelchin commented 2 months ago

Maybe we can find a RA to do this as well.

michaelchin commented 1 month ago

@Hojat-Shirmard let's start with this one. I will send you an email with instructions later.

michaelchin commented 1 month ago

use logger.debug() to print messages in plot_ridges(), plot_ridges(), get_ridges() , get_transforms(), self.ridges property and self.transforms property.

Just to avoid annoying too many people with the warning messages.

The warning messages in other functions are designed to be annoying so that people have the motivations to stop using them.