Skip to content

Some type annotations for coords.py #4294

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Some type annotations for coords.py #4294

wants to merge 5 commits into from

Conversation

rcomer
Copy link
Member

@rcomer rcomer commented Aug 24, 2021 •
edited
Loading

🚀 Pull Request

Description

Added some type hints relevant to the changes in #5955

Consult Iris pull request check list

rcomer
rcomer commented Aug 24, 2021
View reviewed changes
rcomer
rcomer commented Aug 24, 2021
View reviewed changes
@rcomer rcomer force-pushed the collapse-2d-string-coord branch from 484a02b to 9a87565 Compare September 6, 2021 10:14
@rcomer
Copy link
Member Author

rcomer commented Sep 6, 2021

Rebased. CI is green. Ready for review 😎

@rcomer rcomer force-pushed the collapse-2d-string-coord branch from 9a87565 to 13a5a12 Compare November 19, 2021 16:49
@rcomer
Copy link
Member Author

rcomer commented Nov 19, 2021

Added some type hints for the method and function I modified, as well as ones required to tidy up PyRight complaints within the collapsed method. The form import numpy.typing as npt is taken from examples in the numpy docs.

@rcomer
Copy link
Member Author

rcomer commented Nov 19, 2021 •
edited
Loading

Some screenshots of the built docs:

now redundant as we turned hints off in the docs at #4510

ndim_property
bounds_property
core_bounds_method
collapsed_method

bjlittle
bjlittle reviewed Jan 5, 2022
View reviewed changes
@rcomer rcomer force-pushed the collapse-2d-string-coord branch from 13a5a12 to 29e312f Compare January 5, 2022 13:27
@rcomer
Copy link
Member Author

rcomer commented Jan 5, 2022 •
edited
Loading

Made some changes to the type hints following a chat with @bjlittle this morning, and taking some inspiration from @jamesp's stubs.

The rendered docs look better:

now redundant as we turned hints off in the docs at #4510

bounds
core_points_and_bounds
collapsed

@rcomer rcomer force-pushed the collapse-2d-string-coord branch from 0361de4 to af84640 Compare May 4, 2022 15:32
@rcomer rcomer force-pushed the collapse-2d-string-coord branch from af84640 to a8186c7 Compare August 24, 2022 12:45
@bjlittle
Copy link
Member

bjlittle commented Oct 5, 2022

@bjlittle get your finger out...

@bjlittle
Copy link
Member

bjlittle commented Oct 5, 2022

@rcomer Fancy doing a rebase?

@rcomer rcomer force-pushed the collapse-2d-string-coord branch from a8186c7 to 668251d Compare October 5, 2022 10:08
@rcomer
Copy link
Member Author

rcomer commented Oct 5, 2022

Now rebased.

@bjlittle it's all on you now 😛

@rcomer rcomer mentioned this pull request Oct 19, 2022
Closed
@rcomer rcomer force-pushed the collapse-2d-string-coord branch from 668251d to 8820c40 Compare November 23, 2022 17:44
@rcomer rcomer force-pushed the collapse-2d-string-coord branch from 8820c40 to 1eebe2a Compare November 1, 2023 15:32
@rcomer rcomer force-pushed the collapse-2d-string-coord branch from 1eebe2a to 4a3c441 Compare November 29, 2023 15:58
@rcomer rcomer force-pushed the collapse-2d-string-coord branch 2 times, most recently from cb44650 to dd13194 Compare May 8, 2024 11:33
@codecov Codecov
Copy link

codecov bot commented May 8, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.78%. Comparing base (44257eb) to head (1a63e4c).
Report is 130 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #4294   +/-   ##
=======================================
  Coverage   89.78%   89.78%           
=======================================
  Files          88       88           
  Lines       23060    23064    +4     
  Branches     5031     5031           
=======================================
+ Hits        20704    20708    +4     
  Misses       1624     1624           
  Partials      732      732

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rcomer rcomer mentioned this pull request May 15, 2024
Merged
@rcomer
Copy link
Member Author

rcomer commented May 15, 2024 •
edited
Loading

See #5955 for a version of this fix without the type-hinting rabbit hole.

@rcomer rcomer force-pushed the collapse-2d-string-coord branch from dd13194 to d6e7f1b Compare August 30, 2024 14:33
@rcomer rcomer changed the title Partial collapse of multi-dimensional string coords Some type annotations for coords.py Aug 30, 2024
@rcomer rcomer marked this pull request as draft August 30, 2024 14:54
@rcomer
Copy link
Member Author

rcomer commented Aug 30, 2024

The bugfix has now been merged in #5955 so I have rebased this to only include the type changes. It seems that mypy is unhappy though and I'm not sure if/when I will be motivated to investigate that.

@rcomer rcomer removed the Type: Bug label Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bjlittle bjlittle bjlittle left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
🚴 Peloton
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rcomer @bjlittle