Skip to content

Follow up PR for changes requested in #445 #451

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

Merged
merged 2 commits into from
Nov 29, 2022
Merged

Follow up PR for changes requested in #445 #451

merged 2 commits into from
Nov 29, 2022

Conversation

ramvikrams
Copy link
Contributor

@ramvikrams ramvikrams commented Nov 27, 2022

  • Closes #xxxx (Replace xxxx with the Github issue number)
  • Tests added: Please use assert_type() to assert the type of any return value

Follow up PR for #445

Added the two value_counts method as told, tried to change the tests but I have some questions here:-

  1. Sir actually when I write test check(assert_type(c1, pd.Series[int]), pd.Series, pd.Series[int])
    it shows this error Type application has too few types (2 expected) [misc] it only works when we write the test like
    this check(assert_type(c1, pd.Series[int]), pd.Series)
  2. and when the above change is made pytest does not work it shows this error type' object is not subscriptable
    and only works when we write the test like check(assert_type(c1, pd.Series), pd.Series) but mypy fails here it says Expression is of type "Series[float]", not "Series[Any]" [assert-type]
  3. @Dr-Irv Sir the recent changes you made in test_pandas.py in line 1443 also shows this error Failed: DID NOT WARN. No warnings of type (<class 'FutureWarni... while running poe pytest

check(assert_type(c, pd.Series), pd.Series)
c1 = df.groupby("Animal")["Max Speed"].value_counts()
c2 = df.groupby("Animal")["Max Speed"].value_counts(normalize=True)
check(assert_type(c1, pd.Series), pd.Series)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be check(assert_type(c1, "pd.Series[int]"), pd.Series, int) and similar for the next one.

Copy link
Contributor Author

@ramvikrams ramvikrams Nov 28, 2022

Choose a reason for hiding this comment

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

done the changes, sir

Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

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

Thanks @ramvikrams

@Dr-Irv Dr-Irv merged commit 7802c69 into pandas-dev:main Nov 29, 2022
@ramvikrams
Copy link
Contributor Author

Thanks @ramvikrams

Welcome sir

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.

2 participants