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

DEPR: Deprecate Series.view #56054

Merged
merged 16 commits into from
Nov 27, 2023
Merged

DEPR: Deprecate Series.view #56054

merged 16 commits into from
Nov 27, 2023

Conversation

phofl
Copy link
Member

@phofl phofl commented Nov 18, 2023

@phofl phofl added the Deprecate Functionality to remove in pandas label Nov 18, 2023
@phofl phofl added this to the 2.2 milestone Nov 18, 2023
@jorisvandenbossche jorisvandenbossche changed the title DEP: Deprecate Series.view DEPR: Deprecate Series.view Nov 20, 2023
Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this one!

@@ -938,6 +938,11 @@ def view(self, dtype: Dtype | None = None) -> Series:
4 2
dtype: int8
"""
warnings.warn(
"Series.view is deprecated and will be removed in a future version.",
Copy link
Member

Choose a reason for hiding this comment

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

Point to astype instead if the goal was to change the dtype?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah

pandas/tests/groupby/test_timegrouper.py Outdated Show resolved Hide resolved
pandas/tests/series/test_constructors.py Outdated Show resolved Hide resolved
@@ -47,6 +47,10 @@
)
import pandas.core.common as com

pytestmark = pytest.mark.filterwarnings(
"ignore:Series.view is deprecated and will be removed in a future version.:FutureWarning" # noqa: E501
Copy link
Member

Choose a reason for hiding this comment

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

This seems a bit a "dangerous" ignore, because we want to be sure the algos itself aren't raising the warning, and algos seems a part of the codebase that is using view() regularly (although already not with Series objects apparently, assuming it is correct no change is needed there)

Copy link
Member Author

Choose a reason for hiding this comment

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

I can ignore them specifically, will update later

phofl and others added 2 commits November 20, 2023 15:34
warnings.warn(
"Series.view is deprecated and will be removed in a future version.",
FutureWarning,
stacklevel=find_stack_level(),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
stacklevel=find_stack_level(),
stacklevel=2,

(see my comment on the ravel PR at #56053 (comment))

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

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

Can you also add a .. deprecated:: 2.2.0 note to the docstring?

# Conflicts:
#	doc/source/whatsnew/v2.2.0.rst
# Conflicts:
#	doc/source/whatsnew/v2.2.0.rst
@phofl
Copy link
Member Author

phofl commented Nov 21, 2023

Updated

Co-authored-by: Joris Van den Bossche <[email protected]>
@phofl
Copy link
Member Author

phofl commented Nov 21, 2023

Any idea why the annoying validate docstring thing is going crazy as soon as I touch the Series.view example?

cc @MarcoGorelli maybe?

@MarcoGorelli
Copy link
Member

I'll take a look tomorrow, but there is a check to verify that it has an example

@phofl
Copy link
Member Author

phofl commented Nov 21, 2023

Yeah that makes sense but I see tons of unrelated errors, that's what's confusing me, not super urgent, thx

@@ -881,6 +881,10 @@ def view(self, dtype: Dtype | None = None) -> Series:
type. The new data type must preserve the same size in bytes as to not
cause index misalignment.

.. deprecated:: 2.2.0
Copy link
Member

Choose a reason for hiding this comment

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

Apparently this block needs to be placed between the short and extended summary, so after the first sentence above.

Maybe somehow the first error is tripping up further checks ...

Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

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

Thanks!

@jorisvandenbossche jorisvandenbossche merged commit 368643f into pandas-dev:main Nov 27, 2023
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deprecate Functionality to remove in pandas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DEPR: Series.view
3 participants