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

fix: Improve linting/editor experience for hera models. #950

Merged

Conversation

DanCardin
Copy link
Contributor

Pull Request Checklist

Description of PR
Today, hera models seem to receive no editor/linter support for the __init__ arguments of hera models.

This seems to be solely from the necessary try/except imports used to support both pydantic v1 and v2. Both mypy and pyright will bail on and not yield useful types when you do this. But it can be hacked around with some more conditionals imports 😅

Today:
Screenshot 2024-01-31 at 2 19 19 PM
Screenshot 2024-01-31 at 2 19 27 PM

With this change:
Screenshot 2024-01-31 at 2 18 27 PM
Screenshot 2024-01-31 at 2 18 36 PM

Copy link

codecov bot commented Jan 31, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (77c50e7) 81.5% compared to head (b5f0c8e) 81.5%.

Files Patch % Lines
src/hera/workflows/_mixins.py 75.0% 0 Missing and 1 partial ⚠️
src/hera/workflows/io.py 91.6% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main    #950   +/-   ##
=====================================
  Coverage   81.5%   81.5%           
=====================================
  Files         52      52           
  Lines       4079    4091   +12     
  Branches     851     851           
=====================================
+ Hits        3325    3337   +12     
  Misses       562     562           
  Partials     192     192           

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

@@ -7,7 +7,7 @@
from typing import Any, Callable, Dict, List, Optional, Type, TypeVar, Union

from hera.auth import TokenGenerator
from hera.shared._pydantic import BaseModel, root_validator
from hera.shared._pydantic import BaseModel, get_fields, root_validator
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this get_fields thing I did mainly to avoid littering all the existing raw usages of __fields__ with # type: ignore because there's version specific linter error I was seeming to get.

Signed-off-by: DanCardin <ddcardin@gmail.com>
@DanCardin DanCardin force-pushed the dc/improve-model-type-checking branch from f5a1a85 to b5f0c8e Compare January 31, 2024 19:34
@DanCardin
Copy link
Contributor Author

The failing py3.8 build gives me no logs, so i wonder if it's actually a transient CI issue?

And the coverage changes, I think are lies. There should be net no line coverage difference, as far as I can tell.

@samj1912 samj1912 added semver:patch A change requiring a patch version bump type:bug A general bug labels Feb 1, 2024
@samj1912 samj1912 enabled auto-merge (squash) February 1, 2024 02:45
@samj1912 samj1912 merged commit d0c132e into argoproj-labs:main Feb 1, 2024
22 of 24 checks passed
@DanCardin DanCardin deleted the dc/improve-model-type-checking branch February 1, 2024 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:patch A change requiring a patch version bump type:bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants