gh-372 : Fixing Series.astype()

[ramvikrams]

• Closes Fix Series.astype() to return a Series[type] dependent on argument #372
• Tests added: Please use assert_type() to assert the type of any return value

I have created this draft PR and I'll keep commiting to it, so you can also check whether the changes are acceptable or not.

[Dr-Irv]

You need to build a table first before doing a PR. Test locally as well before pushing. You have syntax errors.

[ramvikrams]

You need to build a table first before doing a PR. Test locally as well before pushing. You have syntax errors.

Work is still under progress I'll keep commiting I created this draft PR so that you could be updated with the progress

[Dr-Irv]

Work is still under progress I'll keep commiting I created this draft PR so that you could be updated with the progress

OK, but I think you should build the table first before pushing any more code, so I can review the table. Once we have the table looking right, then we can move forward with code.

Also, you should test locally before pushing to avoid CI failures. E.g., poetry run poe test will run all the tests and will give you much faster feedback.

[ramvikrams]

Work is still under progress I'll keep commiting I created this draft PR so that you could be updated with the progress

OK, but I think you should build the table first before pushing any more code, so I can review the table. Once we have the table looking right, then we can move forward with code.

Also, you should test locally before pushing to avoid CI failures. E.g., poetry run poe test will run all the tests and will give you much faster feedback.

I'll post a picture of the table in a 10 mins

[ramvikrams]

Can you please check this table

[Dr-Irv]

Can you please check this table

Can you post the table as a Markdown table, so then it becomes easier to collaborate via my edits?

You need to add numpy types (np.int, np.float, etc.). You need to add the string representations, e.g., "int", "float", in addition to just int and float

I things like datetime.date and datetime.datetime will map to np.datetime64 . You have to experiment (and write associated tests).

I think it is the case that we need to remove some of the values from S1. for example, I think the datetime ones can be removed:

>>> import pandas as pd
>>> import datetime as dt
>>> a=[dt.datetime(2023, 1, 25), dt.datetime(2023, 1, 26)]
>>> a
[datetime.datetime(2023, 1, 25, 0, 0), datetime.datetime(2023, 1, 26, 0, 0)]
>>> s=pd.Series(a)
>>> s
0   2023-01-25
1   2023-01-26
dtype: datetime64[ns]
>>> s.dtype
dtype('<M8[ns]')
>>> type(s.dtype)
<class 'numpy.dtype[datetime64]'>

So in the above, I created a Series, containing python datetime, but pandas converted the dtype to numpy.datetime64, which probably should be added to S1.

Getting this right is not an easy task and you have to experiment to make sure that the types in S1 are the correct set, and that the arguments for astype() (and pd.Series.__new__() for that matter) are all coordinated.

[ramvikrams]

Yeah i'll create a markdown table and do the required changes i'll do these changes tommorow as it's night here

[Dr-Irv]

Make sure there are entries in the left column for each of the types currently in S1

Once we get this table right, then I think we should split this into 2 PR's. One would remove elements from S1 in _typing.pyi . Once that is done, then we can work on astype().

[ramvikrams]

I have updated the table can you please check it
we can delete the following from S1 :

datetime.date,
datetime.datetime,
datetime.time,
datetime.timedelta,
Timestamp,
Timedelta,
Period,
Interval[int],
Interval[float],
Interval[Timestamp],
Interval[Timedelta],

[Dr-Irv]

I have updated the table can you please check it we can delete the following from S1 :

datetime.date,
datetime.datetime,
datetime.time,
datetime.timedelta,

The above ones can be deleted because they get converted to numpy types.

Timestamp,
Timedelta,
Period,
Interval[int],
Interval[float],
Interval[Timestamp],
Interval[Timedelta],

We can't delete those because of how the typing works. But we can delete many of the references to Series[Timestamp], Series[Timedelta] and Series[Period]. throughout the stubs. See #520 that I'd like you do first.

[Dr-Irv]

I think the table is right

[ramvikrams]

I think the table is right

ok then i'll start with it

[ramvikrams]

Will we have to remove args like "int" which are in quotes because Flake8 says Quoted annotations should never be used in stubs

[Dr-Irv]

For any of the tests such as:

check(assert_type(s.astype("bool"), "Series[bool]"), pd.Series, np.bool_)

you can remove the need to import Series, by changing it to this form:

check(assert_type(s.astype("bool"), "pd.Series[bool]"), pd.Series, np.bool_)

[Dr-Irv]

This is getting closer, and thanks for all the hard work.

Seeing the issue that DataFrame.astype() has no type for the dtype argument, we need to do a bit more to support that as well. So, can you do the following:

1. Take all of the type combinations that you have in series.pyi and make them into new types in _typing.pyi using the following pattern:

BooleanDtypeArg: TypeAlias = type[bool] | type[np.bool_] | pd.BooleanDtype | Literal["bool"]
IntDtypeArg: TypeAlias = type[_str] | pd.StringDtype | Literal["str"]
#  etc for each set of arguments 

2. Then modify series.pyi to import those aliases and use them in series.pyi, which should shorten up that piece of code.
3. In _typing.pyi, change AsTypeArg to be the union of all of the types created in step 1 PLUS ExtensionDtype
4. Create a new overload that should be last in series.pyi for dtype: ExtensionDtype that returns Series . Note - you may need to add some # type: ignore[misc] in different places because I'm pretty sure mypy will complain about this.
5. To test the argument of dtype: ExtensionDtype, do the following:

from decimal import Decimal
from pandas.tests.extension.decimal import DecimalDtype

check(assert_type(pd.Series([Decimal(x) for x in [1,2,3]]).astype(DecimalDtype()), pd.Series), Decimal)

NOTE: If for some reason either type checker complains about importing DecimalDtype (because it will type check the test code), then you can skip this test for now, but just leave it in commented out with a note that the type checkers aren't liking that import.

6. Add tests in tests/test_frame.py that test DataFrame.astype(). Only need to test for a few of the arguments, but should also check using the dict argument as well.

[ramvikrams]

This is getting closer, and thanks for all the hard work.

thanks sir, i'll do the changes and get back

[ramvikrams]

3. In _typing.pyi, change AsTypeArg to be the union of all of the types created in step 1 PLUS ExtensionDtype

Actually I get this when creating a union
Use PEP 604 union types instead of typing.Union (e.g. "int | str" instead of "Union[int, str]").

[Dr-Irv]

3. In _typing.pyi, change AsTypeArg to be the union of all of the types created in step 1 PLUS ExtensionDtype

Actually I get this when creating a union Use PEP 604 union types instead of typing.Union (e.g. "int | str" instead of "Union[int, str]").

Follow the pattern in _typing.pyi and use | to create unions.

[Dr-Irv]

You still need to move all the XXXDtypeArg declarations to right above AstypeArg, in addition to the other comments below

[Dr-Irv]

It appears the CI is failing because we test without pandas being installed.

So go back to commenting out the tests for decimal (and the import), and I will investigate this later.

[ramvikrams]

It appears the CI is failing because we test without pandas being installed.

So go back to commenting out the tests for decimal (and the import), and I will investigate this later.

Done sir

[Dr-Irv]

There are still some unresolved comments

[Dr-Irv]

Please address these comments:

1. gh-372 : Fixing Series.astype() #519 (comment)
2. gh-372 : Fixing Series.astype() #519 (comment)

[Dr-Irv]

You still need to move all the XXXDtypeArg declarations from the bottom of _typing.pyi to right above AstypeArg, in addition to the other comments below

[Dr-Irv]

Any is unacceptable here. Please remove.

[ramvikrams]

Done with all the changes sir

[Dr-Irv]

Thanks @ramvikrams . This was not an easy one to get right, and I anticipate that we may get some feedback when this is released.

[ramvikrams]

Thanks @ramvikrams . This was not an easy one to get right, and I anticipate that we may get some feedback when this is released.

yes if any further change is required i'll do it and thanks sir