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 internal model typing #2517

Merged
merged 18 commits into from
Apr 28, 2024
Merged

Fix internal model typing #2517

merged 18 commits into from
Apr 28, 2024

Conversation

msyyc
Copy link
Member

@msyyc msyyc commented Apr 15, 2024

fixes #2507

The PR includes:

  • tiny refactor to make sure all typing related internal model is correct
  • improve typing of docstring

@msyyc msyyc marked this pull request as ready for review April 15, 2024 08:53
@msyyc msyyc requested a review from tadelesh April 16, 2024 10:26
@msyyc msyyc requested a review from iscai-msft April 24, 2024 07:49
retval = f"{self.code_model.models_filename}.{retval}"
return retval if is_operation_file else f'"{retval}"'
skip_quote = kwargs.get("skip_quote", False)
retval = typing_name(
Copy link
Contributor

Choose a reason for hiding this comment

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

i think it's messier to , because looking at this code, it's a bunch of information that goes into a black box with .

I would be ok with either duplicating this code, or having a helper function that is much simpler, like add_models_file_to_path

Copy link
Member Author

Choose a reason for hiding this comment

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

Fine. To make it clear, I remove the helper function

@msyyc msyyc requested a review from iscai-msft April 26, 2024 06:38
@msyyc msyyc merged commit 1d708d1 into main Apr 28, 2024
14 checks passed
@msyyc msyyc deleted the fix-internal-model-typing branch April 28, 2024 02:45
@msyyc
Copy link
Member Author

msyyc commented Apr 28, 2024

Face is waiting for the fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enum used in csv formatted query is not imported correctly.
3 participants