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

Add version api #2985

Merged
merged 9 commits into from
Dec 6, 2017
Merged

Add version api #2985

merged 9 commits into from
Dec 6, 2017

Conversation

Yancey1989
Copy link
Contributor

@Yancey1989 Yancey1989 commented Jul 20, 2017

Fixed #2924
We can use the version API as:

>>> print paddle.version.version
0.10.0
>>> print paddle.version.git_commit
e1ac448c3320d8a11051fad63cc17f3803862236

Copy link
Collaborator

@wangkuiyi wangkuiyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a serious concern about our current versioning approach, which takes all Git commits between 0.9.0 and 0.10.0 as 0.9.0. This is not realistic.

Consider that when errors occur with an experiment, users should report and upload the log so could we help. The printed log should contain the real version/commit id, so could we reproduce the error and locate the bug.

The correct logic should be

if paddle.version.tagged:
   print paddle.version         # 0.10.0, 0.10.0rc1 or something
   print paddle.version.major   # 0
   print paddle.version.minor   # 10
   print paddle.version.release # 0
   print paddle.version.rc      # 1, or 0 if not a release candidate
else:
   print paddle.version        # 1232131abdd3  
   print paddle.version.commit # 1232131abdd3

What do you think?

@Yancey1989
Copy link
Contributor Author

Yancey1989 commented Jul 21, 2017

Thanks for @wangkuiyi , to distinguish taged or nor is a good idea, I only have some small doubt:

  1. paddle.version is a Python module, so we can use paddle.version.full_version?
  2. Following Semantic Versioning 2.0.0, the version number is made up by MAJOR.MINOR.PATCH
  3. If not a taged version, maybe we do not need to print the repeated information, only print the commit id?

So how about the following logic?

if paddle.version.tagged:
   print paddle.version.full_version         # 0.10.0, 0.10.0rc1 or something
   print paddle.version.major   # 0
   print paddle.version.minor   # 10
   print paddle.version.patch # 0
   print paddle.version.rc      # 1, or 0 if not a release candidate
else:
   print paddle.version.commit # 1232131abdd3

@wangkuiyi
Copy link
Collaborator

@Yancey1989 Completely agree! Thanks!

@Yancey1989
Copy link
Contributor Author

Done with @wangkuiyi 's comments.

Copy link
Contributor

@typhoonzero typhoonzero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Yancey1989 Can you please update this PR and merge the latest changes, so we can merge this.



需要注意的是:

* `release/版本号`分支一旦建立,一般不允许再从`develop`分支合入`release/版本号`。这样保证`release/版本号`分支功能的封闭,方便测试人员测试PaddlePaddle的行为。
* 在`release/版本号`分支存在的时候,如果有bugfix的行为,需要将bugfix的分支同时merge到`master`, `develop`和`release/版本号`这三个分支。

# PaddlePaddle 分支规范
## PaddlePaddle 分支规范
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With markdownlint MD025, there is only one top-level header.

@typhoonzero
Copy link
Contributor

Seem CI failed at:

File "setup.py", line 46, in write_version_py
with open(filename, 'w') as f:
IOError: [Errno 2] No such file or directory: '$/paddle/python/paddle/version.py'

@Yancey1989
Copy link
Contributor Author

@typhoonzero , I'm debugging with this failed and will update this PR later.

Copy link
Contributor

@typhoonzero typhoonzero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! This is a awesome feature.

@Yancey1989 Yancey1989 merged commit c4599d3 into PaddlePaddle:develop Dec 6, 2017
@Yancey1989 Yancey1989 deleted the version_api branch December 6, 2017 11:15
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 this pull request may close these issues.

3 participants