DEPR: Deprecate Series.view by phofl · Pull Request #56054 · pandas-dev/pandas (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation17 Commits16 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

closes DEPR: Series.view #20251 (Replace xxxx with the GitHub issue number)
Tests added and passed if fixing a bug or adding a new feature
All code checks passed.
Added type annotations to new arguments/methods/functions.
Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

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!

dtype: int8
"""
warnings.warn(
"Series.view is deprecated and will be removed in a future version.",

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?

Choose a reason for hiding this comment

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

Yeah

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

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)

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

Co-authored-by: Joris Van den Bossche jorisvandenbossche@gmail.com

warnings.warn(
"Series.view is deprecated and will be removed in a future version.",
FutureWarning,
stacklevel=find_stack_level(),

Choose a reason for hiding this comment

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

stacklevel=find_stack_level(),
stacklevel=2,

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

Choose a reason for hiding this comment

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

Done

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

Co-authored-by: Joris Van den Bossche jorisvandenbossche@gmail.com

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

cc @MarcoGorelli maybe?

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

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

type. The new data type must preserve the same size in bytes as to not
cause index misalignment.

.. deprecated:: 2.2.0

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 ...

Conflicts:

pandas/tests/io/test_sql.py

Choose a reason for hiding this comment

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

Thanks!

DEPR: Deprecate Series.view by phofl · Pull Request #56054 · pandas-dev/pandas (original) (raw)

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Conflicts:

doc/source/whatsnew/v2.2.0.rst

Conflicts:

doc/source/whatsnew/v2.2.0.rst

Choose a reason for hiding this comment

Conflicts:

pandas/tests/io/test_sql.py

Choose a reason for hiding this comment

Labels