fixes #1783

[AdityaRaj23]

• I have added the tests to cover my changes.
• I have updated the documentation and CHANGELOG accordingly.
• I have read the CONTRIBUTING document.

[CLAassistant]

All committers have signed the CLA.

[AdityaRaj23]

I've submitted a pull request, and I'd appreciate it if you could review it to ensure that I haven't unintentionally violated any rules. My changes were limited to the code, and I didn't make any other modifications.

[kessler-frost]

Hey @AdityaRaj23! Thanks for this, you might wanna update the CHANGELOG.md file briefly explaining what did you change under the appropriate section, which for this one is the "Fixed" section, also can you remove the yarn.lock change that was pushed in this one as well. Let me know if you need any help anywhere.

[AdityaRaj23]

should i leave the code i changed commented or remove it before making another pull request?

[kessler-frost]

You don't have to make another pull request, you can just commit and push to this branch itself and it'll be updated here.

[kessler-frost]

@AdityaRaj23 , it seems like you have deleted the yarn.lock file which isn't what I quite meant, I meant that it shouldn't be different than what it currently is in our develop branch.

One tip for the commit messages is that they should be a bit more informative as to what was done in that particular commit. Making only one commit with fixed AgnostiqHQ#1783 is sufficient to connect this PR to the 1783 issue.

[AdityaRaj23]

Sorry, I read the contributing.md there it was written ,for fixing you should commit with message 'fixes#{issue no}' so i stick to that but will surely keep this in my mind from now onwards for any commit

[codecov]

Codecov Report

Merging #1810 (0f6e054) into develop (889bf3e) will decrease coverage by 1.59%.
Report is 4 commits behind head on develop.
The diff coverage is 94.09%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1810      +/-   ##
===========================================
- Coverage    80.17%   78.58%   -1.59%     
===========================================
  Files          232      128     -104     
  Lines        10239     7257    -2982     
  Branches       193        0     -193     
===========================================
- Hits          8209     5703    -2506     
+ Misses        1897     1554     -343     
+ Partials       133        0     -133     

Flag	Coverage Δ		*Carryforward flag
Dispatcher	81.90% <ø> (-4.47%)	⬇️	Carriedforward from 1365650
Functional_Tests	?
SDK	77.71% <94.09%> (+2.88%)	⬆️
UI_Backend	?
UI_Frontend	?

*This pull request uses carry forward flags. Click here to find out more.

[kessler-frost]

Hey @AdityaRaj23 , seems like the test coverage isn't up to the mark with what is expected, so you'll need to add a test for the changes that you made. This will also make sure that the thing that you fixed will not be broken by any of the later commits by anyone else.

So, the file where you'll wanna add the test to is tests/covalent_tests/shared_files/utils_test.py, and the function that needs to be tested is get_named_params. A simpler way to reproduce the same error above is:

from covalent._shared_files.utils import get_named_params

def test_func(a, *, b):
    return a + b

named_args, named_kwargs = get_named_params(test_func, [1], {'b': 2})

So, you essentially just need to test that this isn't failing and then the PR should be gtg.

[AdityaRaj23]

Ok will do as directed

[kessler-frost]

Hey @AdityaRaj23 just bumping this one. You're almost there and I think it would be a shame if this didn't get merged for Hacktoberfest as this is pretty much done. You just need to put the test that you wrote in a function and that would be it.

[AdityaRaj23]

Hey @AdityaRaj23 just bumping this one. You're almost there and I think it would be a shame if this didn't get merged for Hacktoberfest as this is pretty much done. You just need to put the test that you wrote in a function and that would be it.

I would do that today positively was busy in some other work ...

[AdityaRaj23]

tests are fine on my side what is the issue with this debian 10 and debian 11 can you brief me about this i am not sure whats wrong...

[kessler-frost]

You can see the reason why the test failed here: https://github.com/AgnostiqHQ/covalent/actions/runs/6705590122/job/18231396245?pr=1810. The failing statement is assert named_args == [1] whereas it should be assert named_args == {'a': 1} instead. This is since everything will get an appropriate name from running get_named_params.

[kessler-frost]

@AdityaRaj23 Thanks for this, we'll get this merged now.

[AdityaRaj23]

@AdityaRaj23 Thanks for this, we'll get this merged now.

Initially, I mistook this for a different issue. I want to express my gratitude for the valuable assistance I received, as it has been a significant learning experience for me. I've gained a wealth of knowledge, thank you @kessler-frost you've provided me with substantial insights into open source contributions. If an opportunity arises, I would be eager to collaborate and continue learning from you in the future on some other issue on the same project if i worked on it.

[kessler-frost]

Thanks @AdityaRaj23 ! I'm glad you'd like to contribute more we're always looking for eager contributors like you. Please feel free to take a look at issues tagged with good-first-issue or help-wanted labels if you're interested.