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

Simplify Code.attributes type hint #195

Merged

Conversation

phackstock
Copy link
Contributor

closes #191.

Mostly self explanatory, since Code.attributes does no longer need to support more complex region aggregation related data structures we can simplify the type hint.

@danielhuppmann
Copy link
Member

Just to be sure, this typehint has no functional implications, right?

@phackstock
Copy link
Contributor Author

Just to be sure, this typehint has no functional implications, right?

In the pure python sense this does not have any implications. By using pydantic it can have some functional implications. In the order of definition of options for data types in Union, pydantic will attempt type conversion.

As an example if we have:

from pydantic import BaseModel

class A(BaseModel):
    a: Union[int, str]

instance_A = A(a = "3") 
print(type(A.a)) >>> "int"

pydantic converts the string "3" into the integer 3.

This might have actually had a functional use in the past where we had attributes such as check-aggregate in the attributes object. Now that this is only used to hold additional, non-nomenclature functional information, it should not have any functional implications.

Copy link
Member

@danielhuppmann danielhuppmann left a comment

Choose a reason for hiding this comment

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

Thanks @phackstock, looks good!

@phackstock
Copy link
Contributor Author

Seems like resolving the merge conflicts blew up something, I'll investigate.

@phackstock phackstock merged commit ea02177 into IAMconsortium:main Nov 22, 2022
@phackstock phackstock deleted the feature/simplify-code-type-hint branch November 22, 2022 15:22
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.

Simplify Code.attributes type hint
2 participants