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

[DOC] Slightly improve the LineCollection docstring #26676

Merged
merged 1 commit into from
Sep 12, 2023

Conversation

timhoffm
Copy link
Member

@timhoffm timhoffm commented Sep 1, 2023

PR summary

Closes #26674. There's two aspects to the issue:

  • the LineCollection description is slightly unclear. - This is fixed here.

  • Line3DCollection inherits __init__ and reuses its docstring. That's problematic because the the description is 2D. I don't see a good solution here. We likely don't want to replicate the whole docstring, and also a comment like "This holds analogously for 3D with points (x0, y0, z0) and Mx3 arrays." is awkward both for the 2D and 3D docstrings.

    I therefore propose to leave it at this. If the 2D description is clear, the extension to 3D can be inferred by the user. - Not ideal, but the least bad option IMHO.

@timhoffm timhoffm added the Documentation: API files in lib/ and doc/api label Sep 1, 2023

or the equivalent numpy array with two columns. Each line
or the equivalent Mx2 numpy array with two columns. Each line
Copy link
Contributor

@anntzer anntzer Sep 1, 2023

Choose a reason for hiding this comment

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

(M, 2)

I would perhaps completely drop the "list of points" and just write

A sequence ``[line0, line1, ...]`` where each line is a (N, 2)-shape
array-like of points::

    line0 = [(x0, y0), (x1, y1), ...]

Each line can...

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 don't really care. This is a very quick PR to improve a misunderstanding in #26676. I have no time to bike shed this further. Feel free to push to this PR or take over if you think it's possible/worth improving further.

@jklymak
Copy link
Member

jklymak commented Sep 2, 2023

If the 2D description is clear, the extension to 3D can be inferred by the user. - Not ideal, but the least bad option IMHO.

We can always write a 3D docstring, can't we?

@timhoffm
Copy link
Member Author

timhoffm commented Sep 3, 2023

If the 2D description is clear, the extension to 3D can be inferred by the user. - Not ideal, but the least bad option IMHO.

We can always write a 3D docstring, can't we?

Yes. And thinking again, maybe that'd be the sane thing to do, even though that means writing an empty __init__.

Anybody, this PR is just a quick fix for a misleading wording in LineCollection. I'm not going to bike shed this furher or add the 3D docstring. IMHO this can go in as-is as an improvement. Please take over for further improvements.

@oscargus oscargus added this to the v3.8.0 milestone Sep 12, 2023
@oscargus oscargus merged commit 48e9ec7 into matplotlib:main Sep 12, 2023
41 checks passed
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Sep 12, 2023
oscargus added a commit that referenced this pull request Sep 12, 2023
…676-on-v3.8.x

Backport PR #26676 on branch v3.8.x ([DOC] Slightly improve the LineCollection docstring)
@timhoffm timhoffm deleted the doc-linecollection branch September 14, 2023 10:21
@ksunden ksunden mentioned this pull request Sep 15, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation: API files in lib/ and doc/api
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Doc]: Line3DCollection segments
5 participants