lmfit / asteval

minimalistic evaluator of python expression using ast module
https://lmfit.github.io/asteval
MIT License
183 stars 41 forks source link

Backward incompatible change is not documented #96

Closed miso-belica closed 3 years ago

miso-belica commented 3 years ago

Hi, the commit, https://github.com/newville/asteval/commit/26d39b7ec439716faeaf585df5f3f39e84d16676 has implemented suggestion from https://github.com/newville/asteval/issues/88#issuecomment-808895503 but I can' see it in the documentation at https://newville.github.io/asteval/api.html#asteval.eval, nor in the changelog. Also, the version was bumped just by patch number but at least a minor number should be increased because the change is in the API.

Can you please fix it? Thank you.

newville commented 3 years ago

@miso-belica PR for documenting the change is welcome.

Also, the version was bumped just by patch number but at least a minor number should be increased because the change is in the API.

Um ... no. As for any project, the developers and people who contribute to a project decide on the version numbers. People who do not contribute are free to make all the demands on free software they like. Such demands from non-contributors are likely to be ignored if not explicitly refused, and may not be the wisest way to try to get actual results.

miso-belica commented 3 years ago

OK, I assumed every project uses https://semver.org/ and it's a good practice to update minor numbers for API changes for 0.x.y releases so people know something could break after an update. But thanks for letting me know. I will add a note that asteval is not following any convention here :)

miso-belica commented 3 years ago

I created PR with doc update, but I can't update the releases, so please update them :)

newville commented 3 years ago

The API did not break in a backward incompatible way. The old calls work. Yes, the behavior changed, but that is to be expected for any change in the code. The entire premise of your demand that the version number be changed in a way you think it should be is simply wrong.

On Mon, Jul 26, 2021 at 9:04 AM Mišo Belica @.***> wrote:

OK, I assumed every project uses https://semver.org/ and it's a good practice to update minor numbers for API changes for 0.x.y releases so people know something could break after an update. But thanks for letting me know. I will add a note that asteval is not following any convention here :)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/newville/asteval/issues/96#issuecomment-886732081, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACKI64HSJYOV75AM3TNGITTZVTOJANCNFSM5A7USDCA .

-- --Matt Newville 630-327-7411

miso-belica commented 3 years ago

I believe exceptions are part of the api. If some method/function throws exception and stop throwing it it's incompatible change.

Imagine this code. The change breaks it = it's incompatible.

def is_code_valid(code):
  try:
    eval(code, show_errors=False)
    return True
  except Exception:
    return False

I would expect it in changelog, that's all. No matter if the project uses convention for the version or it is incremented randomly like here. I hope you see it now. If not, it's ok. I have comment in my dependencies that the project is fragile so i check the changes in git next time I upgrade 🙂

My suggestion is to document the changes here. At least in documentation, that's why I sent PR. But as I wrote I can't change it in releases so it's up to you unfortunately.

newville commented 3 years ago

@miso-belica

I believe exceptions are part of the api.

I sort of don't. Code changes can add or change the nature (type, message) of exceptions without affecting the API. The API is the syntax of the functions and their return values. It is not the result produced by that call.

Changing the API means that previous code using the library will not be valid. Changing the behavior of the functions - which will happen in every change - means that the results of the function/method call might be (but are not necessarily) different compared to an earlier version. Sure, some changes will have some impact, and changes about when and which situations raise exceptions should be made carefully and done rarely. Here, we had a reasonable request to change how errors propagate, one of the more challenging aspects of this library (ie, how to handle exceptions in the user code in the calling code). But, in contrast to the title of the issue, that change is backward compatible.

Sure, we could do a better job at documenting. Welcome to the world, and thanks for helping out our imperfect corner of it.

I am sorry that you find it unfortunate that you cannot change the release, and that doing so is up to me. Surely, you know much more about this sort of thing than anyone else - that must be frustrating for you.

newville commented 3 years ago

@miso-belica thanks very much for the two thumbs-down responses. Small projects like this, with a small number (here, one!) of all-volunteer developers and averaging something like 1 commit per week over the past 8 years, are always in need of helpful feedback. Such feedback gives motivation for what changes might be needed, who might be interested in using this work, and who might be interested in working on such a project.

You certainly are not obligated to use this code. No warranty was given. The code is freely available in a format that is convenient for many. We try to make it useful but we're under no illusion that it will be suitable for everyone. If you are unhappy with the code or it does not meet your needs or standards, that is actually acceptable to us. Maybe other people will find the code useful.

miso-belica commented 3 years ago

I wrote my feedback with my reasoning above. In return, I got sarcastic comments. All I wanted is to document the changes and that is still missing. I am not going to continue in this useless sarcastic conversation. I have no energy for internet trolls. Bye