nncf icon indicating copy to clipboard operation
nncf copied to clipboard

[Good First Issue] [NNCF] Make NNCF common utils code pass mypy checks

Open vshampor opened this issue 1 year ago • 23 comments

This is exactly like https://github.com/openvinotoolkit/nncf/issues/2495 (see the description and tasks there), but the target code path for this one is nncf/common/utils instead of nncf/common/graph.

vshampor avatar Jan 25 '24 15:01 vshampor

.take

tvilight4 avatar Jan 25 '24 16:01 tvilight4

Thank you for looking into this issue! Please let us know if you have any questions or require any help.

github-actions[bot] avatar Jan 25 '24 16:01 github-actions[bot]

.take

ishan121028 avatar Feb 02 '24 15:02 ishan121028

Thanks for being interested in this issue. It looks like this ticket is already assigned to a contributor. Please communicate with the assigned contributor to confirm the status of the issue.

github-actions[bot] avatar Feb 02 '24 15:02 github-actions[bot]

I am currently working on the issue. Facing a few problems but trying to solve them..will give the updates in a few days

tvilight4 avatar Feb 03 '24 19:02 tvilight4

Hello @tvilight4, is there anything we can help you with?

p-wysocki avatar Feb 19 '24 14:02 p-wysocki

@p-wysocki I am new to type hinting and so figuring out the solution is taking time, Do the location of the function calls matter in the codebase? If yes it would be great if you could tell me where the function is_tensorflow_model(model: TModel) being called...as nncf/common/utils/backend.py:73: error: Cannot find implementation or library stub for module named "tensorflow" is the first mypy error which i am trying to solve.

tvilight4 avatar Feb 20 '24 20:02 tvilight4

Hello @tvilight4, usually the best way to fix mypy errors is to google them - quite often you even can find copy and paste fixes for the errors. It seems that particular one could be changed by modifying the import line: https://stackoverflow.com/questions/68695851/mypy-cannot-find-implementation-or-library-stub-for-module

p-wysocki avatar Feb 23 '24 13:02 p-wysocki

Hello @tvilight4, do you need any help?

p-wysocki avatar Mar 01 '24 13:03 p-wysocki

@p-wysocki I a working on it but will need time till 20 march due to my university exams going on.

tvilight4 avatar Mar 02 '24 12:03 tvilight4

Hello @tvilight4, are you still working on this?

p-wysocki avatar Apr 03 '24 18:04 p-wysocki

Sorry @p-wysocki I have not been able to work on this issue due to other urgent commitments. I would opt to unassign myself for now so that someone else can take it up. If I get some solution, I will inform you. I am extremely sorry for the delay and inconvenience caused.

tvilight4 avatar Apr 04 '24 05:04 tvilight4

Don't worry, there are no deadlines in Good First Issues, we're just looking for abandoned tasks for other people to pick up. :) For now I'm reopening the task, but feel free to pick up something else or even this one from our board as soon as you have some time!

cc @vshampor could you please reopen the task?

p-wysocki avatar Apr 05 '24 07:04 p-wysocki

I would like to take this issue when it opens @p-wysocki

anzr299 avatar Apr 09 '24 01:04 anzr299

.take

anzr299 avatar Apr 09 '24 04:04 anzr299

Thanks for being interested in this issue. It looks like this ticket is already assigned to a contributor. Please communicate with the assigned contributor to confirm the status of the issue.

github-actions[bot] avatar Apr 09 '24 04:04 github-actions[bot]

.take

anzr299 avatar Apr 18 '24 08:04 anzr299

Thank you for looking into this issue! Please let us know if you have any questions or require any help.

github-actions[bot] avatar Apr 18 '24 08:04 github-actions[bot]

@alexsu52 I am trying to define the _registry_dict dictionary in the Registry class. I have decided to go ahead with a general object type for both the keys and values. I am facing some errors in nncf\common\graph\patterns\manager.py wherein the expected type is Dict[HWFusedPatternNames, Callable[[], GraphPattern] for the registry dict. Casting it to the type expected from the general object typing works. I was wondering if casting would be accepted as a solution?

anzr299 avatar Apr 18 '24 09:04 anzr299

@anzr299, casting is applicable solution for this case. Let's discuss this in more detail in PR.

alexsu52 avatar Apr 19 '24 10:04 alexsu52

Sure, just to clarify, I think the issue referred to by @MaximProshin is #2637.

anzr299 avatar Apr 19 '24 10:04 anzr299

Hello @anzr299, are you still working on that issue? Do you need any help?

p-wysocki avatar May 06 '24 09:05 p-wysocki

Hi @p-wysocki, I was busy for the past few days. I will continue working on this issue now.

anzr299 avatar May 07 '24 20:05 anzr299

@anzr299, thanks for the contribution!

alexsu52 avatar Jul 05 '24 07:07 alexsu52