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

Clarity on backwards compatibility given date-based versioning #1413

Closed
dwinston opened this issue Mar 12, 2019 · 9 comments
Closed

Clarity on backwards compatibility given date-based versioning #1413

dwinston opened this issue Mar 12, 2019 · 9 comments
Labels
discussion pkg Package health and distribution related stuff

Comments

@dwinston
Copy link
Member

#213 clarified that backwards-incompatible changes used to be signaled with the bumping of the so-called minor version, e.g. someone upgrading pymatgen 3.3.x to 3.4.y should expect the possibility of breaking changes and should consult the change log. However, since v2017.6.8, pymatgen has switched to date-based versioning. This is a problem because it makes every upgrade something for users to be cautious about. I am raising this issue now because a user asked me when they would "need" to upgrade next, and I don't know the answer.

Ideally, one release will have "long-term support" in that backwards-compatible bug fixes will be applied. Ubuntu handles this with date-based versioning by pledging long-term support of an April release each two years (e.g. 18.04 for the Apr 2018 release, 16.04 for the 2016 release, etc.).

I don't know the right path for pymatgen. Perhaps pymatgen could release backwards-incompatible changes only in each year's January release? Perhaps this is already the case, but I'd like this issue to be closed only on this being stated explicitly. Perhaps backwards-incompatible changes have already been introduced in released versions since v2019.1.13, in which case perhaps v2019.4 can be the version with only backwards-compatible changes until v2020.4.

@mkhorton
Copy link
Member

In terms of backwards incompatibility, the most important thing is that v2019.x onwards is Python 3 only, which was announced both in the docs and in the mailing list.

Generally, I think pymatgen has been fairly good about backwards compatibility and deprecations. Have you noticed specific breaking changes that have caused problems, or have any been reported to you? Might be useful for discussion to have a specific example in mind.

@dwinston
Copy link
Member Author

Here's one strange example from the MP discussion forum: https://discuss.materialsproject.org/t/get-entries-returns-empty-list-for-irru3/1976/3. I did not care to investigate what changed because the user's error was using the Nov 6, 2015 release of pymatgen.

The issue is that I expected breaking changes from e.g. 3.2.5 to 3.3.0 because @shyuep declared in #213 a license to break compatibility on minor version bumps. Now, I don't know what to expect going forward.

@shyuep
Copy link
Member

shyuep commented Mar 12, 2019

I would say that most changes are backwards compatible. It is fine if it is something that is understood to be a work in progress, e.g if you are coding a new analysis and is releasing a new version. We expect anyone using these bleeding edge features to adjust. But for the core, all changes are expected to be backwards compatible with a relatively long deprecation schedule (usually a year). That is why I took the step of warning people about 2019 py3 only from late 2017.

No one needs to upgrade as long as they are not using bleeding edge features. They can always install 2018 and we will support them all the same.

@janosh janosh added discussion pkg Package health and distribution related stuff labels Jun 23, 2023
@janosh
Copy link
Member

janosh commented Jun 23, 2023

I personally prefer semantic versioning precisely for the clarity of expectations. In x.y.z, if x changes expect backwards incompatible changes, if y changes expect new features, if z changes expect fixes.

That said, I don't see anything actionable in this issue so closing as completed.

@janosh janosh closed this as completed Jun 23, 2023
@mkhorton
Copy link
Member

I agree, I don't think there's anything imminently actionable here.

That said, I've come around a lot more strongly with regards to semantic versioning since the last time this thread was open. I realise it causes more overhead and work for us, but at a certain point when a package becomes important enough to other people's workflows, it becomes worth the investment.

@shyuep
Copy link
Member

shyuep commented Jun 23, 2023

I would challenge someone to find the last time that an actual backward-incompatible change occurred in pymatgen core classes. I get the theoretical argument for semantic versioning. But if the practical effect is negligible, then the choice is based on what is easiest for maintenance. I see too many proposals on ideal solutions but frequently, the execution is less than disciplined. I would rather spend the time ensuring things like unittest coverage, i.e., things that actually matter in terms of correctness of code.

@janosh
Copy link
Member

janosh commented Jun 23, 2023

I would challenge someone to find the last time that an actual backward-incompatible change occurred in pymatgen core classes.

Today. I normalized the error messages which strictly speaking is a breaking change. If someone built a tool like custodian on top of pymatgen that reacts to certain error messages, that could be broken by today's release. Very unlikely so I think it's fine but you asked. :)

I agree tests are most important. Many of our tests were only checking the error class, not the error message. I found several tests where the expected error message was left as a comment but the actual error raised was not the one in the comment. pymatgen's test coverage has a long ways still to go but at least I managed to test all messages on errors raised in pymatgen.core (a9c3ca3 893cad7 0baff29)

@shyuep
Copy link
Member

shyuep commented Jun 23, 2023

If you are suggesting that based on semantic versioning, you are going to increment the major version number for error messages, that is plainly ridiculous.

@janosh
Copy link
Member

janosh commented Jun 23, 2023

If you take SV seriously, then yes. numpy or pytorch would not change error messages in a minor. I'm not that strict personally. I also don't see the connection between frequency of breaking changes and the discussion on what version scheme to use. SV is better for reasons mentioned above in any case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion pkg Package health and distribution related stuff
Projects
None yet
Development

No branches or pull requests

4 participants