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

[bugfix]: Fix type hint errors in Features.py #5556

Merged
merged 5 commits into from
Mar 18, 2024
Merged

[bugfix]: Fix type hint errors in Features.py #5556

merged 5 commits into from
Mar 18, 2024

Conversation

huibosa
Copy link
Contributor

@huibosa huibosa commented Mar 17, 2024

Description

  • Use Union instead of |
  • Squence[float] and NDArray for point

Backporting

This change should be backported to the previous releases, please add the relevant labels.
Refer here to resolve backport conflicts if any.

  • 2016
  • 2017
  • 2018
  • 2019
  • 2020
  • 2021
  • 2022
  • 2023
  • 2024

@keywordlabeler keywordlabeler bot added backport-to-2016 Backport to 2016 branch backport-to-2017 Backport to 2017 branch backport-to-2018 Backport to 2018 branch backport-to-2019 Backport to 2019 branch backport-to-2020 Backport to 2020 branch backport-to-2021 Backport to 2021 branch backport-to-2022 Backport to 2022 branch backport-to-2023 Backport to 2023 branch bugfix labels Mar 17, 2024
@keywordlabeler keywordlabeler bot added the backport-to-2024 Backport to 2024 branch label Mar 17, 2024
Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @huibosa - I've reviewed your changes and they look great!

General suggestions:

  • Verify the consistency and appropriateness of default values for parameters now accepting NDArray to ensure they align with the expected types.
  • Consider adding runtime validation for parameters accepting a wide range of types to ensure they are compatible with the method's operations.
  • Review the handling of NDArray inputs in methods, especially for operations involving vector arithmetic or transformations, to prevent potential bugs.
Here's what I looked at during the review
  • 🟡 General issues: 3 issues found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Docstrings: all looks good

Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨

Share Sourcery

Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

src/abaqus/Feature/Feature.py Outdated Show resolved Hide resolved
src/abaqus/Feature/Feature.py Show resolved Hide resolved
@huibosa
Copy link
Contributor Author

huibosa commented Mar 17, 2024

Seems I have made a mistake on the the type ReferencePoint, this is the first time i make a PR, how do I fix it? Does it needs your action or mine?

@haiiliin haiiliin removed the backport-to-2024 Backport to 2024 branch label Mar 17, 2024
@haiiliin
Copy link
Owner

We use | instead of Union in all places, could you please revert these changes?

@haiiliin
Copy link
Owner

haiiliin commented Mar 18, 2024

The tests fail because the code requires numpy to be installed, however in the test workflow numpy is not installed. I don't want this library to depend on numpy because it is just for typing, could you move the numpy imports from numpy.typing import NDArray to the if TYPE_CHECKING block so that the library does not depend on numpy and the numpy import will only be evaluated on type checking:

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from numpy.typing import NDArray

Copy link
Owner

@haiiliin haiiliin left a comment

Choose a reason for hiding this comment

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

Can you revise the PR according to the comments?

src/abaqus/Feature/Feature.py Show resolved Hide resolved
src/abaqus/Feature/Feature.py Outdated Show resolved Hide resolved
src/abaqus/Feature/Feature.py Outdated Show resolved Hide resolved
@haiiliin
Copy link
Owner

haiiliin commented Mar 18, 2024

Seems I have made a mistake on the the type ReferencePoint, this is the first time i make a PR, how do I fix it? Does it needs your action or mine?

I don't know why ReferencePoint causes the mypy pre-commit hook to fail, please revise according the above comments first.

@haiiliin
Copy link
Owner

The docs error has nothing to do with this PR, merging

@haiiliin haiiliin merged commit 74fc5c1 into haiiliin:2024 Mar 18, 2024
6 of 8 checks passed
@haiiliin
Copy link
Owner

@huibosa Thanks for your contribution

mergify bot pushed a commit that referenced this pull request Mar 18, 2024
* [bugfix]: Fix type hint errors

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [revert]: Use '|' instead of Union

* [revert]: undo some unwanted changes

* Fix pre-commit errors

---------

Co-authored-by: huibosa_macmini <huibosa@email.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Hailin Wang <hailin.wang@connect.polyu.hk>
(cherry picked from commit 74fc5c1)
@haiiliin haiiliin linked an issue Mar 18, 2024 that may be closed by this pull request
mergify bot pushed a commit that referenced this pull request Mar 18, 2024
* [bugfix]: Fix type hint errors

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [revert]: Use '|' instead of Union

* [revert]: undo some unwanted changes

* Fix pre-commit errors

---------

Co-authored-by: huibosa_macmini <huibosa@email.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Hailin Wang <hailin.wang@connect.polyu.hk>
(cherry picked from commit 74fc5c1)
mergify bot pushed a commit that referenced this pull request Mar 18, 2024
* [bugfix]: Fix type hint errors

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [revert]: Use '|' instead of Union

* [revert]: undo some unwanted changes

* Fix pre-commit errors

---------

Co-authored-by: huibosa_macmini <huibosa@email.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Hailin Wang <hailin.wang@connect.polyu.hk>
(cherry picked from commit 74fc5c1)
mergify bot added a commit that referenced this pull request Mar 18, 2024
[bugfix]: Fix type hint errors in Features.py (#5556)

* [bugfix]: Fix type hint errors

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [revert]: Use '|' instead of Union

* [revert]: undo some unwanted changes

* Fix pre-commit errors

---------

Co-authored-by: huibosa_macmini <huibosa@email.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Hailin Wang <hailin.wang@connect.polyu.hk>
(cherry picked from commit 74fc5c1)

Co-authored-by: huibosa <78707242+huibosa@users.noreply.github.com>
mergify bot added a commit that referenced this pull request Mar 18, 2024
[bugfix]: Fix type hint errors in Features.py (#5556)

* [bugfix]: Fix type hint errors

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [revert]: Use '|' instead of Union

* [revert]: undo some unwanted changes

* Fix pre-commit errors

---------

Co-authored-by: huibosa_macmini <huibosa@email.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Hailin Wang <hailin.wang@connect.polyu.hk>
(cherry picked from commit 74fc5c1)

Co-authored-by: huibosa <78707242+huibosa@users.noreply.github.com>
mergify bot added a commit that referenced this pull request Mar 18, 2024
[bugfix]: Fix type hint errors in Features.py (#5556)

* [bugfix]: Fix type hint errors

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [revert]: Use '|' instead of Union

* [revert]: undo some unwanted changes

* Fix pre-commit errors

---------

Co-authored-by: huibosa_macmini <huibosa@email.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Hailin Wang <hailin.wang@connect.polyu.hk>
(cherry picked from commit 74fc5c1)

Co-authored-by: huibosa <78707242+huibosa@users.noreply.github.com>
mergify bot pushed a commit that referenced this pull request Mar 18, 2024
* [bugfix]: Fix type hint errors

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [revert]: Use '|' instead of Union

* [revert]: undo some unwanted changes

* Fix pre-commit errors

---------

Co-authored-by: huibosa_macmini <huibosa@email.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Hailin Wang <hailin.wang@connect.polyu.hk>
(cherry picked from commit 74fc5c1)
mergify bot pushed a commit that referenced this pull request Mar 18, 2024
* [bugfix]: Fix type hint errors

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [revert]: Use '|' instead of Union

* [revert]: undo some unwanted changes

* Fix pre-commit errors

---------

Co-authored-by: huibosa_macmini <huibosa@email.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Hailin Wang <hailin.wang@connect.polyu.hk>
(cherry picked from commit 74fc5c1)
mergify bot pushed a commit that referenced this pull request Mar 18, 2024
* [bugfix]: Fix type hint errors

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [revert]: Use '|' instead of Union

* [revert]: undo some unwanted changes

* Fix pre-commit errors

---------

Co-authored-by: huibosa_macmini <huibosa@email.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Hailin Wang <hailin.wang@connect.polyu.hk>
(cherry picked from commit 74fc5c1)
mergify bot added a commit that referenced this pull request Mar 18, 2024
[bugfix]: Fix type hint errors in Features.py (#5556)

* [bugfix]: Fix type hint errors

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [revert]: Use '|' instead of Union

* [revert]: undo some unwanted changes

* Fix pre-commit errors

---------

Co-authored-by: huibosa_macmini <huibosa@email.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Hailin Wang <hailin.wang@connect.polyu.hk>
(cherry picked from commit 74fc5c1)

Co-authored-by: huibosa <78707242+huibosa@users.noreply.github.com>
mergify bot added a commit that referenced this pull request Mar 18, 2024
[bugfix]: Fix type hint errors in Features.py (#5556)

* [bugfix]: Fix type hint errors

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [revert]: Use '|' instead of Union

* [revert]: undo some unwanted changes

* Fix pre-commit errors

---------

Co-authored-by: huibosa_macmini <huibosa@email.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Hailin Wang <hailin.wang@connect.polyu.hk>
(cherry picked from commit 74fc5c1)

Co-authored-by: huibosa <78707242+huibosa@users.noreply.github.com>
mergify bot added a commit that referenced this pull request Mar 18, 2024
[bugfix]: Fix type hint errors in Features.py (#5556)

* [bugfix]: Fix type hint errors

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [revert]: Use '|' instead of Union

* [revert]: undo some unwanted changes

* Fix pre-commit errors

---------

Co-authored-by: huibosa_macmini <huibosa@email.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Hailin Wang <hailin.wang@connect.polyu.hk>
(cherry picked from commit 74fc5c1)

Co-authored-by: huibosa <78707242+huibosa@users.noreply.github.com>
mergify bot pushed a commit that referenced this pull request Mar 18, 2024
* [bugfix]: Fix type hint errors

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [revert]: Use '|' instead of Union

* [revert]: undo some unwanted changes

* Fix pre-commit errors

---------

Co-authored-by: huibosa_macmini <huibosa@email.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Hailin Wang <hailin.wang@connect.polyu.hk>
(cherry picked from commit 74fc5c1)
mergify bot pushed a commit that referenced this pull request Mar 18, 2024
* [bugfix]: Fix type hint errors

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [revert]: Use '|' instead of Union

* [revert]: undo some unwanted changes

* Fix pre-commit errors

---------

Co-authored-by: huibosa_macmini <huibosa@email.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Hailin Wang <hailin.wang@connect.polyu.hk>
(cherry picked from commit 74fc5c1)
mergify bot added a commit that referenced this pull request Mar 18, 2024
[bugfix]: Fix type hint errors in Features.py (#5556)

* [bugfix]: Fix type hint errors

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [revert]: Use '|' instead of Union

* [revert]: undo some unwanted changes

* Fix pre-commit errors

---------

Co-authored-by: huibosa_macmini <huibosa@email.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Hailin Wang <hailin.wang@connect.polyu.hk>
(cherry picked from commit 74fc5c1)

Co-authored-by: huibosa <78707242+huibosa@users.noreply.github.com>
mergify bot added a commit that referenced this pull request Mar 18, 2024
[bugfix]: Fix type hint errors in Features.py (#5556)

* [bugfix]: Fix type hint errors

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [revert]: Use '|' instead of Union

* [revert]: undo some unwanted changes

* Fix pre-commit errors

---------

Co-authored-by: huibosa_macmini <huibosa@email.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Hailin Wang <hailin.wang@connect.polyu.hk>
(cherry picked from commit 74fc5c1)

Co-authored-by: huibosa <78707242+huibosa@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-2016 Backport to 2016 branch backport-to-2017 Backport to 2017 branch backport-to-2018 Backport to 2018 branch backport-to-2019 Backport to 2019 branch backport-to-2020 Backport to 2020 branch backport-to-2021 Backport to 2021 branch backport-to-2022 Backport to 2022 branch backport-to-2023 Backport to 2023 branch bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Willing to make a PR, and I have some questions
2 participants