Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Added__round__ method for vectors#3559

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

Already on GitHub?Sign in to your account

Merged
MyreMylar merged 1 commit intopygame:mainfromMatiiss:matiiss-add-vector-round
Dec 16, 2022
Merged

Added__round__ method for vectors#3559

MyreMylar merged 1 commit intopygame:mainfromMatiiss:matiiss-add-vector-round
Dec 16, 2022

Conversation

@Matiiss
Copy link
Contributor

@MatiissMatiiss commentedNov 13, 2022
edited
Loading

Starbuck5 reacted with hooray emoji
@Matiiss
Copy link
ContributorAuthor

Why is this failing on a font test?

@Matiiss
Copy link
ContributorAuthor

What? Those tests didn't fail this time.

@Starbuck5
Copy link
Contributor

This will need a note in the documentation that this was added in pygame 2.1.4.

@Matiiss
Copy link
ContributorAuthor

Matiiss commentedNov 13, 2022
edited
Loading

This will need a note in the documentation that this was added in pygame 2.1.4.

That's what I thought, the question is where in the docs should this be added? I suppose it's not exactly a separate method, because it's not intended to be called likevector_instance.__round__(ndigits), so how and where? A note under this:Changed in pygame 1.9.4:pygame.math required import. More convenient pygame.Vector2 and pygame.Vector3.:
image
?

Perhaps, something like this becauseround signature should be already known or does it require an example:
image

Also, I just realized that that line aboutrequired import is a bit confusing, isrequired import what changed in 1.9.4 (as in, is no more required) or is it required as of 1.9.4 (because AFAIK it's not required)?

@Matiiss
Copy link
ContributorAuthor

Matiiss commentedNov 13, 2022
edited
Loading

Should this support keyword arguments?

@Starbuck5
Copy link
Contributor

Should this support keyword arguments?

It should try to emulate a normal numerical__round__. So if they do, we should.

The docs addition you bring up makes sense.

@Matiiss
Copy link
ContributorAuthor

Matiiss commentedNov 13, 2022
edited
Loading

__round__ for floats doesn't support keyword arguments,round on the other hand does already support keyword arguments but it's a built-in function so those keyword arguments can't really be changed, either way, it seems to work fine with keyword arguments too

It is a bit odd to do this thoughround(number=vector_instance),ndigits keyword works fine too.

@Matiiss
Copy link
ContributorAuthor

Should the docs say that this is new in version 2.1.4.dev1?

@oddbookworm
Copy link
Contributor

oddbookworm commentedNov 17, 2022
edited
Loading

Should the docs say that this is new in version 2.1.4dev1

I think it'd be fine saying just 2.1.4

Copy link
Contributor

@Starbuck5Starbuck5 left a comment

Choose a reason for hiding this comment

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

math.rst line 16 has a description of all the numerical operations Vectors support, it would be good to add support forround to that list.

Copy link
Contributor

@Starbuck5Starbuck5 left a comment

Choose a reason for hiding this comment

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

Left a couple more comments for consideration before merge, but the code and functionality looks great. Good job!

@Matiiss
Copy link
ContributorAuthor

Did I just...

Copy link
Contributor

@MyreMylarMyreMylar left a comment

Choose a reason for hiding this comment

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

needs a few memory leak fixes. See#3590 for more details.

+ tests (taken fromhttps://github.com/python/cpython/blob/c32bc1bffd9d63ede0d0505abab983247a3ad0c6/Lib/test/test_builtin.py#L1477)Fixes#3523Added type stubsAdded `__round__` method for vectorsFix FASTCALL issues by replacing with METH_VARARGSFixed weird tabAdded change to docsTabs to spacesUpdate math.cSeparated `||` into 2 `if`s, removed an unreasonable check since `ndigits` will equal -1 if user provides that value.Update math.rstAdded `round` to numerical operation listFixed conflictMinor `pygame.math` doc enhancementsDoc additionsAdded `Py_DECREF(ret);`
@Matiiss
Copy link
ContributorAuthor

@MyreMylar I have made the requested changes, it would be great if you could review them, thanks!

Copy link
Contributor

@MyreMylarMyreMylar left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Matiiss reacted with thumbs up emojiMatiiss reacted with hooray emojiMatiiss reacted with rocket emoji
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

3 more reviewers

@Starbuck5Starbuck5Starbuck5 approved these changes

@ankith26ankith26ankith26 left review comments

@MyreMylarMyreMylarMyreMylar approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

mathpygame.mathNew APIThis pull request may need extra debate as it adds a new class or function to pygame

Projects

None yet

Milestone

2.2.0

Development

Successfully merging this pull request may close these issues.

__round__ method forpygame.Vector2 andpygame.Vector3

5 participants

@Matiiss@Starbuck5@oddbookworm@MyreMylar@ankith26

[8]ページ先頭

©2009-2025 Movatter.jp