API: change default behavior in `Table.pformat` and `Column.pformat` and add (pending) deprecation to `Table.pformat_all` by neutrinoceros · Pull Request #17184 · astropy/astropy

neutrinoceros · 2024-10-15T18:04:43Z

Description

EDIT: Closes #17179

By checking this box, the PR author has requested that maintainers do NOT use the "Squash and Merge" button. Maintainers should respect this when possible; however, the final decision is at the discretion of the maintainer that merges the PR.

github-actions · 2024-10-15T18:05:15Z

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

Do the proposed changes actually accomplish desired goals?
Do the proposed changes follow the Astropy coding guidelines?
Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

github-actions · 2024-10-15T18:05:17Z

👋 Thank you for your draft pull request! Do you know that you can use [ci skip] or [skip ci] in your commit messages to skip running continuous integration tests until you are ready?

taldcroft

Looks great, thanks! I just have some word-smithing suggestions. I think this should also get a What's new entry.

taldcroft · 2024-10-15T19:52:13Z

+            -1 (the default) implies no truncation
+            pass None to ensure the output doesn't exceed the limits
+            of current terminal.


For this and other similar docstring changes:

Suggested change

-1 (the default) implies no truncation

pass None to ensure the output doesn't exceed the limits

of current terminal.

-1 (default) implies no limit, ``None`` implies using the

height of the current terminal.

taldcroft · 2024-10-15T19:54:25Z


+    @deprecated(
+        since="7.0",
+        message="We are planning on deprecating Table.pformat_all",


Suggested change

message="We are planning on deprecating Table.pformat_all",

message="Table.pformat_all will be deprecated",

the phrasing I used is borrowed from the only other pending deprecation (Table.show_in_browser). I'm also not sure it's worth doing a pending deprecation if we're 100% sure that it will be deprecated; why not just deprecate it now then ?

(I'm going to assume a direct deprecation is okay for this round of review, let me know if you want me to change it back)

taldcroft

Just some little documentation nits now, otherwise all good!

taldcroft · 2024-10-22T17:52:51Z

Needs a rebase. :-(

neutrinoceros · 2024-10-22T19:20:45Z

suggestions integrated and branch rebased !

taldcroft · 2024-10-23T15:50:26Z

Sorry, this needs a rebase as well. But now I have auto-merge enabled so it should go through!

…add (pending) deprecation to Table.pformat_all

pllim · 2024-10-23T17:21:41Z

@taldcroft , you need to approve for auto merge to happen. 😉

neutrinoceros · 2024-10-23T17:24:13Z

Something seems stuck, all required checks are green but auto-merge still didn't happen. I'll try to disable it an merge manually.

Update: whoops, once again, beaten by @pllim 😀

pllim · 2024-10-23T17:26:46Z

Well, if you keep the auto-merge, it would have merged as soon as approval is granted.

taldcroft

Looks good!

github-actions Bot added table io.ascii labels Oct 15, 2024

neutrinoceros added this to the v7.0.0 milestone Oct 15, 2024

neutrinoceros force-pushed the table/depr/deprecate_pformat_all branch 2 times, most recently from b36726e to f881699 Compare October 15, 2024 18:21

neutrinoceros changed the title ~~API: change default behavior in Table.pformat and Column.pformat and add (pending) deprecation to Table.pformat_all~~ API: change default behavior in Table.pformat and Column.pformat and add (pending) deprecation to Table.pformat_all Oct 15, 2024

neutrinoceros force-pushed the table/depr/deprecate_pformat_all branch from f881699 to 9b1db24 Compare October 15, 2024 18:23

neutrinoceros marked this pull request as ready for review October 15, 2024 18:48

neutrinoceros requested review from dhomeier and taldcroft as code owners October 15, 2024 18:48

neutrinoceros mentioned this pull request Oct 15, 2024

DEPR: add a pending deprecation to Table.pformat's default values on arguments max_lines and max_width #17179

Closed

1 of 3 tasks

taldcroft added whatsnew-needed API change PRs and issues that change an existing API, possibly requiring a deprecation period labels Oct 15, 2024

taldcroft reviewed Oct 15, 2024

View reviewed changes

neutrinoceros force-pushed the table/depr/deprecate_pformat_all branch from edde4df to e18a719 Compare October 16, 2024 07:48

neutrinoceros removed the whatsnew-needed label Oct 16, 2024

neutrinoceros force-pushed the table/depr/deprecate_pformat_all branch from e18a719 to 21fbd75 Compare October 16, 2024 08:32

taldcroft reviewed Oct 22, 2024

View reviewed changes

Comment thread astropy/table/column.py Resolved

Comment thread astropy/table/column.py Resolved

Comment thread docs/whatsnew/7.0.rst Resolved

neutrinoceros force-pushed the table/depr/deprecate_pformat_all branch from 222455d to 3a9d4cf Compare October 22, 2024 19:20

taldcroft enabled auto-merge (squash) October 23, 2024 15:49

neutrinoceros force-pushed the table/depr/deprecate_pformat_all branch from 3a9d4cf to d852be3 Compare October 23, 2024 16:01

API: change default behavior in Table.pformat and Column.pformat and …

778022f

…add (pending) deprecation to Table.pformat_all

neutrinoceros force-pushed the table/depr/deprecate_pformat_all branch from d852be3 to 778022f Compare October 23, 2024 16:17

neutrinoceros disabled auto-merge October 23, 2024 17:24

taldcroft approved these changes Oct 24, 2024

View reviewed changes

taldcroft merged commit 04d2a63 into astropy:main Oct 24, 2024

neutrinoceros deleted the table/depr/deprecate_pformat_all branch October 24, 2024 14:28

	message="We are planning on deprecating Table.pformat_all",
	message="Table.pformat_all will be deprecated",

Conversation

neutrinoceros commented Oct 15, 2024 • edited by pllim Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

github-actions Bot commented Oct 15, 2024

Uh oh!

github-actions Bot commented Oct 15, 2024

Uh oh!

taldcroft left a comment

Choose a reason for hiding this comment

Uh oh!

taldcroft Oct 15, 2024

Choose a reason for hiding this comment

Uh oh!

taldcroft Oct 15, 2024

Choose a reason for hiding this comment

Uh oh!

neutrinoceros Oct 16, 2024

Choose a reason for hiding this comment

Uh oh!

neutrinoceros Oct 16, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

taldcroft left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

taldcroft commented Oct 22, 2024

Uh oh!

neutrinoceros commented Oct 22, 2024

Uh oh!

taldcroft commented Oct 23, 2024

Uh oh!

pllim commented Oct 23, 2024

Uh oh!

neutrinoceros commented Oct 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pllim commented Oct 23, 2024

Uh oh!

taldcroft left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

neutrinoceros commented Oct 15, 2024 •

edited by pllim

Loading

neutrinoceros commented Oct 23, 2024 •

edited

Loading