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

ENH: Adding pd.__git_version__ to point to git sha commit (#21295) #21680

Closed
wants to merge 10 commits into from

Conversation

atulagrwl
Copy link

@@ -83,6 +83,7 @@
from ._version import get_versions
v = get_versions()
__version__ = v.get('closest-tag', v['version'])
__git_version__ = v['full-revisionid']
Copy link
Contributor

Choose a reason for hiding this comment

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

this won’t work for released version
use .get

pls add a test for this

Copy link
Author

Choose a reason for hiding this comment

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

Adding test to check if it returns valid sha should be okay?

@codecov
Copy link

codecov bot commented Jun 30, 2018

Codecov Report

Merging #21680 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #21680   +/-   ##
=======================================
  Coverage   92.05%   92.05%           
=======================================
  Files         170      170           
  Lines       50708    50708           
=======================================
  Hits        46677    46677           
  Misses       4031     4031
Flag Coverage Δ
#multiple 90.45% <ø> (ø) ⬆️
#single 42.36% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a2fbce...9421725. Read the comment docs.

@jreback
Copy link
Contributor

jreback commented Jul 1, 2018

we have a once a day CI - https://github.com/pandas-dev/pandas-ci
that does a full install via pip and conda and then runs tests like a user
i believe this will fail here (these are post merge tests)

as git_version would be None

can u verify?

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

pls add a release note in 0.24 other enhancements



def test_git_version():
git_version = pd.__git_version__
Copy link
Contributor

Choose a reason for hiding this comment

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

can u add the issue number here

@pep8speaks
Copy link

pep8speaks commented Jul 1, 2018

Hello @atulagrwl! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on July 27, 2018 at 01:32 Hours UTC

@atulaggarwal
Copy link

atulaggarwal commented Jul 1, 2018

we have a once a day CI - https://github.com/pandas-dev/pandas-ci
that does a full install via pip and conda and then runs tests like a user
i believe this will fail here (these are post merge tests)

I am not sure how to do that. This is my first commit to pandas and would appreciate if you tell me help me.

@jreback jreback added the Build Library building on various platforms label Jul 7, 2018
# Conflicts:
#	doc/source/whatsnew/v0.24.0.txt
@atulagrwl
Copy link
Author

Merged latest master to resolve conflict in whatsnew file.

@@ -262,3 +264,10 @@ def test_compression_warning(compression_only):
check_stacklevel=False):
with f:
df.to_csv(f, compression=compression_only)


# GH 21295
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Pls put comment below def line.

@jbrockmendel
Copy link
Member

@atulagrwl one more rebase, almost over the finish line.

@atulagrwl
Copy link
Author

@jbrockmendel - Did the rebase and minor refactoring as suggested in test.

@@ -180,6 +180,8 @@ Other Enhancements
- :func:`~DataFrame.to_csv` and :func:`~DataFrame.to_json` now support ``compression='infer'`` to infer compression based on filename (:issue:`15008`)
- :func:`to_timedelta` now supports iso-formated timedelta strings (:issue:`21877`)
- :class:`Series` and :class:`DataFrame` now support :class:`Iterable` in constructor (:issue:`2193`)
- New method :meth:`__git_version__` will return git commit sha of current build
Copy link
Member

Choose a reason for hiding this comment

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

This is an attribute, not a method, right?

@jbrockmendel
Copy link
Member

@jreback looks about right.


def test_git_version():
# GH 21295
git_version = pd.__git_version__
Copy link
Contributor

Choose a reason for hiding this comment

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

This will fail when built (which we test on pandas-ci), as the git_version will be None.

Copy link
Contributor

Choose a reason for hiding this comment

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

does this actually run when pandas is installed after tagging?
iow need to tag (locally) then build a release and install and pd.test()

Copy link
Member

@alimcmaster1 alimcmaster1 Sep 18, 2018

Choose a reason for hiding this comment

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

Looks like it will, I ran:
python setup.py bdist_wheel
and installed that wheel in a fresh virtual env.
pd.__git_version__ then return the git sha of commit

@@ -83,6 +83,7 @@
from ._version import get_versions
v = get_versions()
__version__ = v.get('closest-tag', v['version'])
__git_version__ = v.get('full-revisionid')
Copy link
Contributor

Choose a reason for hiding this comment

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

is this what dask does? this will be None in the built version I think

Choose a reason for hiding this comment

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

Is there a way I can create a built version locally and test it out? @jreback

Copy link
Contributor

Choose a reason for hiding this comment

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

python setup.py bdist_wheel, and then install that wheel in a new env.

Choose a reason for hiding this comment

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

Created wheel from above command (https://www.dropbox.com/s/3ieyl07ctcsja9u/pandas-0.24.0.dev0%2B373.g942172546-cp36-cp36m-macosx_10_7_x86_64.whl?dl=0) and it showed git_version correctly

>>> import pandas as pd
>>> pd.__version__
'0.24.0.dev0+373.g942172546'
>>> pd.__git_version__
'9421725461061b55bc84e388d3952957d9b0cd83'

@@ -180,6 +180,8 @@ Other Enhancements
- :func:`~DataFrame.to_csv` and :func:`~DataFrame.to_json` now support ``compression='infer'`` to infer compression based on filename (:issue:`15008`)
- :func:`to_timedelta` now supports iso-formated timedelta strings (:issue:`21877`)
- :class:`Series` and :class:`DataFrame` now support :class:`Iterable` in constructor (:issue:`2193`)
- New attribute :attr:`__git_version__` will return git commit sha of current build
Copy link
Contributor

Choose a reason for hiding this comment

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

add the issue ref here

@alimcmaster1
Copy link
Member

@atulagrwl this is almost there, mind merging in the latest changes to these files? And updating the issue ref as per @jreback comment.

@alimcmaster1
Copy link
Member

Im happy to take this over the line, since we haven't heard back from @atulagrwl . Will reopen a new PR and we can close this.

CC. @jangorecki

@alimcmaster1 alimcmaster1 mentioned this pull request Sep 18, 2018
@jschendel
Copy link
Member

Closing in favor of #22745

@jschendel jschendel closed this Sep 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Library building on various platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

access git revision of currently running pandas
8 participants