Skip to content
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

Interpretation of argument for use_legacy_defaults is not clear #432

Closed
murrayrm opened this issue Jul 18, 2020 · 3 comments · Fixed by #435
Closed

Interpretation of argument for use_legacy_defaults is not clear #432

murrayrm opened this issue Jul 18, 2020 · 3 comments · Fixed by #435
Milestone

Comments

@murrayrm
Copy link
Member

My sense of the way that use_legacy_defaults should work is that if I had code that was working in some version and it stops working in a future version, I should be able to use use_legacy_defaults to return things to the version that was working.

So, for example, if I had things working in 0.8.2 and I upgrade to 0.8.4 or 0.9.0 and things stop working, I should be able to say

use_legacy_defaults('0.8.2')

and my code will work again.

This particular call definitely won't work (use_legacy_defaults only recognizes '0.8.3') and it is not clear to me how it could work. Basically, it seems like we need to track when a change in default behavior occurred and then "revert" any defaults that are different between the version I was using and the current version.

@sawyerbfuller Does my issue make sense? This is related to PR #431, I think.

@murrayrm murrayrm added this to the 0.8.4 milestone Jul 18, 2020
@sawyerbfuller
Copy link
Contributor

I don’t have much familiarity with backwards-incompatible changes before 0.8.3 but I assumed that we could use this function to add such functionality as needed from feedback from users.

My take on it is that the function should be available as a “good enough” solution that allows users to avoid the thorniest issues that come up whenever there are big changes in the code (a few of which I anticipated in that PR). But not something that will include every version number and provide compatibility with minor revision of how the code works.

In short, we should be a little bit “opinionated” about how things ought to work rather than to aim to maintain perfect backwards compatibility at all times.

So, if I update to the latest version of python-control and something breaks in my code, the first thing I should try is use-legacy-defaults on various previous version numbers, one of which might fix things, but maybe not everything.

@sawyerbfuller
Copy link
Contributor

sawyerbfuller commented Jul 18, 2020

We could alternatively include every recent release in that function, most of which would set the same defaults as each other.

@murrayrm
Copy link
Member Author

I agree with the philosophy, especially the summary sentence:

So, if I update to the latest version of python-control and something breaks in my code, the first thing I should try is use-legacy-defaults on various previous version numbers, one of which might fix things, but maybe not everything.

But this means that use_legacy_defaults needs to take an arbitrary version number and do the "right thing", which I don't think it currently does. Eg, if I were using 0.8.2 and things were working, then I got to 0.8.4 and things break, in the current implementation I would need to know to call use_legacy_defaults('0.8.3') even though I never personally used 0.8.3.

I think a bit of clever parsing of the version number and lexicographical comparison would allow an implementation where if I enter a version number I get the legacy defaults for anything that changed after that version number.

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

Successfully merging a pull request may close this issue.

2 participants