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.

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

Define metadata return types #97

Merged
merged 2 commits into from
Aug 29, 2024
Merged

Define metadata return types #97

merged 2 commits into from
Aug 29, 2024

Conversation

axelboc
Copy link
Contributor

@axelboc axelboc commented Aug 29, 2024

Addresses #96 (comment)

@axelboc axelboc requested a review from loichuder August 29, 2024 08:10
t20100
t20100 reviewed Aug 29, 2024
View reviewed changes
Copy link
Member

@t20100 t20100 left a comment

Choose a reason for hiding this comment

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

Typing can be simplified: Since python3.7 with an "magic" import: from __future__ import annotations, you can use types directly without importing and also use |

h5grove/content.py Outdated Show resolved Hide resolved
h5grove/content.py Outdated Show resolved Hide resolved
h5grove/content.py Outdated Show resolved Hide resolved
h5grove/models.py Show resolved Hide resolved
h5grove/models.py Show resolved Hide resolved
h5grove/models.py Outdated Show resolved Hide resolved
axelboc
axelboc commented Aug 29, 2024
View reviewed changes
h5grove/models.py Outdated Show resolved Hide resolved
@axelboc axelboc force-pushed the types branch 4 times, most recently from d5af3c1 to 71ad84b Compare August 29, 2024 09:38
@axelboc
Copy link
Contributor Author

axelboc commented Aug 29, 2024

Seems pytest doesn't like the new syntax: https://github.com/silx-kit/h5grove/actions/runs/10612574960/job/29414512350?pr=97

Should I do what it says and install eval_type_backport?

@loichuder
Copy link
Member

Seems pytest doesn't like the new syntax: https://github.com/silx-kit/h5grove/actions/runs/10612574960/job/29414512350?pr=97

Should I do what it says and install eval_type_backport?

Seems like we opened the Pandora's box 😂

Yes, you can add eval-type-backport to the dev extras_require.

@axelboc
Copy link
Contributor Author

axelboc commented Aug 29, 2024

Phew... 😅

t20100
t20100 approved these changes Aug 29, 2024
View reviewed changes
Copy link
Member

@t20100 t20100 left a comment

Choose a reason for hiding this comment

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

💪

loichuder
loichuder approved these changes Aug 29, 2024
View reviewed changes
h5grove/models.py Outdated Show resolved Hide resolved
test/base_test.py Show resolved Hide resolved
@loichuder
Copy link
Member

Speaking of which, it would be nice to also add tests for more recent Python versions in the CI.

@axelboc
Copy link
Contributor Author

axelboc commented Aug 29, 2024

Speaking of which, it would be nice to also add tests for more recent Python versions in the CI.

That I will leave to you 😂

@axelboc axelboc merged commit 116a629 into main Aug 29, 2024
1 check passed
@axelboc axelboc deleted the types branch August 29, 2024 12:22
@loichuder loichuder mentioned this pull request Aug 29, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@loichuder loichuder loichuder approved these changes

@t20100 t20100 t20100 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@axelboc @loichuder @t20100