Avaiga / taipy-core

A Python library to build powerful and customized data-driven back-end applications.
Apache License 2.0
38 stars 17 forks source link

Remove Type ignore from DataNode.storageType() method #417

Closed jrobinAV closed 1 year ago

jrobinAV commented 1 year ago

The storageType method must return a str and not an Optional[str]. This is incompatible with the current implementation of the _class_map method

ashutuptiwari commented 1 year ago

I am assuming you're referring to this method image in the file src/taipy/core/data/data_node.py. Can you explain the changes that are needed and assign this issue to me @jrobinAV

trgiangdo commented 1 year ago

If you remove the # type: ignore, mypy will complain that:

Incompatible return value type (got "Type[NotImplementedError]", expected "str")

The purpose of this issue is to safely remove the # type: ignore and still follow mypy style guideline.

ashutuptiwari commented 1 year ago

Alright I'll get started @trgiangdo . Please assign this issue to me @jrobinAV

ashutuptiwari commented 1 year ago

If you remove the # type: ignore, mypy will complain that:

Incompatible return value type (got "Type[NotImplementedError]", expected "str")

The purpose of this issue is to safely remove the # type: ignore and still follow mypy style guideline.

What command is leading to this error log? @trgiangdo

trgiangdo commented 1 year ago

You can try to remove the # type: ignore and run this command on the terminal:

mypy --ignore-missing-imports --implicit-optional .
ashutuptiwari commented 1 year ago

Thanks!

ashutuptiwari commented 1 year ago

i got this error 2 times , but not the one that you mentioned

 error: No overload variant of "asdict" matches argument type "_BaseModel"  [call-overload]

@trgiangdo

trgiangdo commented 1 year ago

The full error message that you are looking for:

src/taipy/core/data/data_node.py:314: error: Incompatible return value type (got "Type[NotImplementedError]", expected "str")  [return-value]

If it's not showing up, maybe we are missing something.

ashutuptiwari commented 1 year ago

Yeah that one isn't there image @trgiangdo

ashutuptiwari commented 1 year ago

@trgiangdo ?

trgiangdo commented 1 year ago

Can you show me:

ashutuptiwari commented 1 year ago

Sure, what command should i use for python @trgiangdo ? The mypy version is giving the same result as before Here is the ss of the code you asked for image

trgiangdo commented 1 year ago

Can you try to remove the # type: ignore from line 314? Then the error I mentioned should show up.

ashutuptiwari commented 1 year ago

Both these solution seem to resolve this error @trgiangdo

  1. image

  2. image

ashutuptiwari commented 1 year ago

@trgiangdo

trgiangdo commented 1 year ago

Sorry for the late response, I was away for the weekend.

About your fix, it's contradictory with the purpose of the storage_type() method. It should always return the NotImplemented exception to avoid the user from using the _DataNode class directly, or a child class without the storage type would raise an error.

Please find another work-around

ashutuptiwari commented 1 year ago

image This seems to work as well @trgiangdo

ashutuptiwari commented 1 year ago

Hacktoberfest will be ending tomorrow so it will be really helpful if we could close this issue before then @trgiangdo . Please review this solution

ashutuptiwari commented 1 year ago

Hey, i think there has been some mistake image because this repository is shown as not participating in hacktoberfest 2023. Please look into it. @trgiangdo @jrobinAV @FlorianJacta

ashutuptiwari commented 1 year ago

IMG-20231030-WA0045.jpg

Please confirm @trgiangdo

FlorianJacta commented 1 year ago

@ashutuptiwari The warning should go away now. Can you try again?

ashutuptiwari commented 1 year ago

Yes it did @FlorianJacta . Thanks a bunch!

ashutuptiwari commented 1 year ago

I guess this issue can be closed?

trgiangdo commented 1 year ago

We will review it and close done issue on every sprint. The merged PR should be sufficient for the hacktoberfest, no?

ashutuptiwari commented 1 year ago

Oh yes it is. I was just curious.

ashutuptiwari commented 11 months ago

Hi guys, I was wondering if i would recieve some sticker packs or some swag image @FabienLelaquais @jrobinAV @FlorianJacta @marisogo

marisogo commented 11 months ago

Hi @ashutuptiwari, can you shoot me an email at: marine.gosselin@taipy.com? So we can organize sending you the swag :)