Deprecate module level functions #229

tomschr · 2020-03-15T20:49:38Z

This PR fixes #225.

For example, the VersionInfo.bump_major() method calls semver.parse_version_info and semver.bump_major instead of having its own implementation. This has several drawbacks:

the VersionInfo class depends on other, "remote" function outside the class.
the class is not "self-contained".
it makes deprecating module level functions (and removing them) harder because of these dependencies.

As such, I had to move some implementation into the VersionInfo class. With this approach, we can deprecate the module level function without triggering a false positive when calling a method in VersionInfo.

This PR contains the following changes:

Add test cases
- Test also missing finalize_version
- Test the warning more thouroughly with pytest.warns instead of just pytest.deprecated_call
In setup.cfg, add deprecation warnings filter for pytest
Implement DeprecationWarning with warnings module and the new decorator deprecated
Output a DeprecationWarning for the following functions:
- semver.bump_{major,minor,patch,prerelease,build}
- semver.finalize_version
- semver.format_version
- semver.parse
- semver.parse_version_info
- semver.replace
Move module level implementation into VersionInfo
- Move implementations:
  - semver.format_version -> ~~semver.VersionInfo.format_version~~ semver.VersionInfo.__str__
  - Move _REGEX into VersionInfo class
  - Move _LAST_NUMBER into VersionInfo class
  - Move also _increment_string and implement it as a staticmethod
  - semver.parse -> semver.VersionInfo.parse
  - semver.bump_* -> semver.VersionInfo.bump_*
- Change implementation by calling VersionInfo methods (and not the other way around):
  - semver.compare
  - semver.finalize_version
  - semver.replace
  - semver.parse_version_info
Introduce new public functions:
- semver.VersionInfo.to_dict (from former _asdict)
- semver.VersionInfo.to_tuple (from former _astuple)
- Keep _asdict and _astuple as a (deprecated) function for compatibility reasons
Update CHANGELOG.rst
Update usage documentation:
- Move some information to make them more useful for the reader
- Add deprecation warning
- Explain how to replace deprecated functions
- Explain how to display deprecation warnings from semver
Improve documentation of deprecated functions
- List deprecated module level functions
- Make recommendation and show equivalent code
- Mention that deprecated functions will be replaced in semver 3. That means, all deprecated function will be still available in semver 2.x.y.

@gsakkis As you were also involved, I'd like to hear your opinion too. 😉

gsakkis

@tomschr looks good in general; just left a few minor comments and suggestions.

semver.py

s-celles · 2020-04-10T06:53:38Z

conflict with semver.py need to be fixed.

tomschr · 2020-04-10T09:05:50Z

@scls19fr I fixed the conflicts now.

There is still two things we need to clarify:

Do we want to deprecate the module level function in the next semver major release (which would be 3) or in major 4? Karl's opinion is to have more time as there are still projects which uses the module level functions.
Anything left from the implementation perspective which could be improved?

I will improve the documentation as I think, this still needs a note or some further explanations.

s-celles · 2020-04-10T09:41:45Z

I have no idea about what is the best... deprecating module function in major 3.x.x or in 4.x.x.

Anyway documenting is the best we can do currently.

tomschr · 2020-04-10T09:45:29Z

@scls19fr

I have no idea about what is the best... deprecating module function in major 3.x.x or in 4.x.x.

No problem. In that case, we could deprecate them in 4.x.y to be on the safe side. That may give projects a little bit more time and we could address @ppkt concerns.

There is another issue which just came into my mind: should we make the _asdict() method "public" (=removing the underscore from the function)?

The reason for this is it could help projects which still uses semver.parse() and using dictionaries. That could ease the migration path.

s-celles · 2020-04-10T09:48:52Z

I'm wondering if todict won't be a better name.

tomschr · 2020-04-10T09:49:48Z

Yep, that's probably a better name. 👍 I will do the change and add also some tests.

s-celles · 2020-04-10T09:49:59Z

or to_dict as in Pandas

s-celles · 2020-04-10T09:51:13Z

see in https://pandas.pydata.org/pandas-docs/stable/reference/frame.html
to_... methods

tomschr · 2020-04-10T09:51:58Z

Ok, even better. I'll use to_dict

tomschr · 2020-04-10T11:21:38Z

You can find my current implementation of semver.VersionInfo.to_dict in commit 9c89a53

s-celles · 2020-04-10T15:11:08Z

I wonder if we shouldn't even have

to_tuple
to_string
methods

so it will be easier to know using tab completion how to transform a VersionInfo
simply by writing VersionInfo.to_<tab>

tomschr · 2020-04-10T15:16:43Z

Actually, I thought of that too. 😄

I think, the to_tuple could be useful. 👍

However, I'm not sure about the to_string method. I think, in this case we have already a solution: str(version). IMHO we shouldn't offer a second way ("There should be one-- and preferably only one --obvious way to do it.").

If you really want it, we can implement it as an alias:

to_string = __str__

s-celles · 2020-04-10T16:39:14Z

ok! let's forget to_string

tomschr · 2020-04-10T17:03:11Z

@scls19fr I've updated the documentation, see for details commit c4ac0ec.

All changes are in section "Using semver". As the changes are a bit spread, I would recommend to build the documentation with tox -e docs and open the HTML file docs/_build/html/usage.html to get an impression.

If you are missing some important information, let me know. After your approval, I would squash the commits to have a clean history.

tomschr · 2020-04-11T09:02:12Z

@scls19fr Thanks for the approval. 👍 However, before I can squash and merge it, I found some other issues which I would like to ask.

In commit 1d8c415, I've moved the _LAST_NUMBER variable and the function _increment_string into the VersionInfo class. IMHO, now the class contains everything that is needed and it is not dependent from another "external" function. Would that be fine for you?

Another question is regarding the warnings. As I've improved the deprecated decorator in commit 08c71eb I'm not sure anymore if I've introduced some issues.

For example, I've tried to display the warnings, however this seems not to work:

$ python3  -c "import semver; print(semver.parse_version_info('3.4.5'))"
3.4.5

However, when I add the -W option with d (=default), then I can see them:

$ python3 -Wd -c "import semver; print(semver.parse_version_info('3.4.5'))"
<string>:1: DeprecationWarning: Function 'semver.parse_version_info' is deprecated. Deprecated since version 2.9.2. Use 'semver.VersionInfo.parse' instead.
3.4.5

@ppkt, @scls19fr do you know if this is an expected behaviour? Is the correct? Not sure...
Any hints highly appreciated! 👍

Thanks for your help!

ppkt

LGTM :)

semver.py

ppkt · 2020-04-11T10:53:24Z

@tomschr yes, it seems that by default DeprecationWarnings are ignored:

Base category for warnings about deprecated features when those warnings are intended for other Python developers (ignored by default, unless triggered by code in __main__).

I'm not sure about meaning of last sentence however

tomschr · 2020-04-11T11:04:24Z

@ppkt Thanks for your analysis. Much appreciated. 👍

yes, it seems that by default DeprecationWarnings are ignored:

Yes, that was also my findings, but I wasn't sure.

Base category for warnings about deprecated features when those warnings are intended for other Python developers (ignored by default, unless triggered by code in __main__).

I'm not sure about meaning of last sentence however

Hmn, maybe they mean something like this:

import semver  # not triggered in the library?

if __name__ == "__main__":
   # Warning is triggered here

Just a theory. I think, I should mention the -W/-Wd option in the documentation to make developers aware of this.

tomschr · 2020-04-11T11:13:45Z

Yes, my suspicion seems to be correct. Here is a short test code:

#!/usr/bin/python3

import warnings
import semver

def fourtytwo():
    warnings.warn("Oh no, we found the answer!", category=UserWarning)
    return 42

if __name__ == "__main__":
  print(semver.bump_major('3.4.5'))
  print(fourtytwo())

When I run this script, I get:

$ ./fortytwo.py
4.0.0
./fortytwo.py:7: UserWarning: Oh no, we found the answer!
  warnings.warn("Oh no, we found the answer!", category=UserWarning)
42

which is exactly the result predicted from the Python documentation.

When I change the shebang line and add the -Wd option, I get all:

$ ./fortytwo.py
./fortytwo.py:11: DeprecationWarning: Function 'semver.bump_major' is deprecated. Deprecated since version 2.9.2.  This function will be removed in semver 3. Use the respective 'semver.VersionInfo.None' instead.
  print(semver.bump_major('3.4.5'))
4.0.0
./fortytwo.py:7: UserWarning: Oh no, we found the answer!
  warnings.warn("Oh no, we found the answer!", category=UserWarning)
42

tomschr · 2020-04-11T12:45:27Z

@scls19fr @ppkt @gsakkis I've squashed the commits and added all necessary information into the commit message.

I'm finished. The documentation is updated, the test cases works and even the coverage report shows 100%! 🎉
(~~However, for some odd reason, Travis and GitHub Actions seems to cease their work. 😞 Looks like GH has some problems.~~)

Could you have a final look if everything is ok? I will merge it soon.

semver.py

tomschr · 2020-04-14T16:03:17Z

As the changes are a bit big, I will leave you some more time. If nobody objects, I will merge it on Thursday. 😉

* Add test cases - Add additional test case for "check" - test_should_process_check_iscalled_with_valid_version - Test also missing finalize_version - Test the warning more thoroughly with pytest.warns instead of just pytest.deprecated_call * In `setup.cfg`, add deprecation warnings filter for pytest * Implement DeprecationWarning with warnings module and the new decorator `deprecated` * Output a DeprecationWarning for the following functions: - semver.bump_{major,minor,patch,prerelease,build} - semver.format_version - semver.finalize_version - semver.parse - semver.parse_version_info - semver.replace - semver.VersionInfo._asdict - semver.VersionInfo._astuple Add also a deprecation notice in the docstrings of these functions * Introduce new public functions: - semver.VersionInfo.to_dict (from former _asdict) - semver.VersionInfo.to_tuple (from former _astuple) - Keep _asdict and _astuple as a (deprecated) function for compatibility reasons * Update CHANGELOG.rst * Update usage documentation: - Move some information to make them more useful for for the reader - Add deprecation warning - Explain how to replace deprecated functions - Explain how to display deprecation warnings from semver * Improve documentation of deprecated functions - List deprecated module level functions - Make recommendation and show equivalent code - Mention that deprecated functions will be replaced in semver 3. That means, all deprecated function will be still available in semver 2.x.y. * Move _increment_string into VersionInfo class - Makes removing deprecating functions easier as, for example, bump_prerelease is no longer dependant from an "external" function. - Move _LAST_NUMBER regex into VersionInfo class - Implement _increment_string as a staticmethod Co-authored-by: Karol <karol@ppkt.eu> Co-authored-by: scls19fr <scls19fr@users.noreply.github.com> Co-authored-by: George Sakkis

tomschr added Release_2.x.y Only for the major release 2 Enhancement Not a bug, but increases or improves in value, quality, desirability, or attractiveness labels Mar 15, 2020

tomschr requested a review from a team March 15, 2020 20:49

tomschr self-assigned this Mar 15, 2020

tomschr force-pushed the feature/deprecationwarning branch from c2a5ce3 to a32df30 Compare March 15, 2020 21:09

tomschr mentioned this pull request Mar 16, 2020

Decide to deprecate module level functions? #225

Closed

gsakkis reviewed Mar 17, 2020

View reviewed changes

semver.py Outdated Show resolved Hide resolved

semver.py Outdated Show resolved Hide resolved

semver.py Outdated Show resolved Hide resolved

semver.py Outdated Show resolved Hide resolved

tomschr force-pushed the feature/deprecationwarning branch 3 times, most recently from 61aeca1 to ecd7017 Compare March 20, 2020 07:08

tomschr mentioned this pull request Apr 9, 2020

str() doesn't work as expected #232

Closed

tomschr force-pushed the feature/deprecationwarning branch 2 times, most recently from c8948ba to 9a43790 Compare April 10, 2020 09:01

s-celles approved these changes Apr 11, 2020

View reviewed changes

ppkt approved these changes Apr 11, 2020

View reviewed changes

semver.py Outdated Show resolved Hide resolved

semver.py Outdated Show resolved Hide resolved

tomschr mentioned this pull request Apr 11, 2020

Remove deprecated functions for semver 3.1.0 (or better semver 4?) #233

Open

tomschr force-pushed the feature/deprecationwarning branch from d4348fa to 561384c Compare April 11, 2020 12:41

ppkt approved these changes Apr 11, 2020

View reviewed changes

semver.py Outdated Show resolved Hide resolved

tomschr force-pushed the feature/deprecationwarning branch 2 times, most recently from 334c6c6 to 09ef970 Compare April 12, 2020 14:42

This was referenced Apr 13, 2020

First implementation of next_version #222

Merged

Idea: Use case for a VersionIterator? #223

Open

tomschr force-pushed the feature/deprecationwarning branch 3 times, most recently from 6069d66 to 4611457 Compare April 14, 2020 09:29

tomschr force-pushed the feature/deprecationwarning branch from 4611457 to 5b8bb16 Compare April 15, 2020 08:25

tomschr merged commit c9b2a7f into python-semver:master Apr 16, 2020

tomschr deleted the feature/deprecationwarning branch April 16, 2020 17:22

tomschr mentioned this pull request Apr 18, 2020

Create new 2.10.0 Release #237

Closed

5 tasks

Deprecate module level functions #229

Deprecate module level functions #229

Uh oh!

Conversation

tomschr commented Mar 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gsakkis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

s-celles commented Apr 10, 2020

Uh oh!

tomschr commented Apr 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

s-celles commented Apr 10, 2020

Uh oh!

tomschr commented Apr 10, 2020

Uh oh!

s-celles commented Apr 10, 2020

Uh oh!

tomschr commented Apr 10, 2020

Uh oh!

s-celles commented Apr 10, 2020

Uh oh!

s-celles commented Apr 10, 2020

Uh oh!

tomschr commented Apr 10, 2020

Uh oh!

tomschr commented Apr 10, 2020

Uh oh!

s-celles commented Apr 10, 2020

Uh oh!

tomschr commented Apr 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

s-celles commented Apr 10, 2020

Uh oh!

tomschr commented Apr 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tomschr commented Apr 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ppkt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ppkt commented Apr 11, 2020

Uh oh!

tomschr commented Apr 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tomschr commented Apr 11, 2020

Uh oh!

tomschr commented Apr 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

tomschr commented Apr 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

tomschr commented Mar 15, 2020 •

edited

Loading

tomschr commented Apr 10, 2020 •

edited

Loading

tomschr commented Apr 10, 2020 •

edited

Loading

tomschr commented Apr 10, 2020 •

edited

Loading

tomschr commented Apr 11, 2020 •

edited

Loading

tomschr commented Apr 11, 2020 •

edited

Loading

tomschr commented Apr 11, 2020 •

edited

Loading

tomschr commented Apr 14, 2020 •

edited

Loading