Fix part of #12912: Changes deprecated python methods #14506

IamEzio · 2021-12-28T18:37:34Z

Overview

This PR fixes part of Fix pylint problems #12912 .
This PR does the following: Removes following deprecated python methods -

Essential Checklist

The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
The linter/Karma presubmit checks have passed locally on your machine.
"Allow edits from maintainers" is checked. (See here for instructions on how to enable it.)
- This lets reviewers restart your CircleCI tests for you.
The PR is made from a branch that's not called "develop".

Proof that changes are correct

PR Pointers

Make sure to follow the instructions for making a code change.
Oppiabot will notify you when you don't add a PR_CHANGELOG label. If you are unable to do so, please @-mention a code owner (who will be in the Reviewers list), or ask on Gitter.
For what code owners will expect, see the Code Owner's wiki page.
Make sure your PR follows conventions in the style guide, otherwise this will lead to review delays.
Never force push. If you do, your PR will be closed.
Oppiabot can assign anyone for review/help if you leave a comment like the following: "{{Question/comment}} @{{reviewer_username}} PTAL"
Some of the e2e tests are flaky, and can fail for reasons unrelated to your PR. We are working on fixing this, but in the meantime, if you need to restart the tests, please check the "If your build fails" wiki page.

oppiabot · 2021-12-28T18:37:47Z

Hi @vojtechjelinek, @seanlip -- could one of you please add the appropriate changelog label to this pull request? Thanks!

vojtechjelinek

Thanks! Left two comments.

CHANGELOG

vojtechjelinek · 2021-12-29T09:11:04Z

core/tests/test_utils.py

+        sig = inspect.signature(self._func)
+
+        args_dict = sig.bind_partial(*args, **kwargs)


Why this change?

getcallargs() was also deprecated in the new python version and was showing the same error as other deprecated methods when pylint ran.

Okay, could you then do

Suggested change

sig = inspect.signature(self._func)

args_dict = sig.bind_partial(*args, **kwargs)

sig = inspect.signature(self._func)

args_dict = sig.bind_partial(*args, **kwargs)

and add an explanatory comment above it explaining what this code does.

Sorry, but I made a mistake here as this would not be the correct way to replace inspect.getcallargs( ). The method getcallargs() returns a dict while bind_partial( ) returns BoundArguments. This test is failing due to this issue.

I searched for the valid solution of this issue but could not find any satisfactory solution except these lines of code which use bind( ) to return dict.

However I am not sure of this and open to suggestions.
This error didn't show up in my localmachine. I am sorry if I caused any troubles.

@vojtechjelinek PTAL

@vojtechjelinek Can you please help here?

@IamEzio Hey look at this https://docs.python.org/3/library/inspect.html#inspect.BoundArguments, the structure of BoundArguments is explained there, from this you should be able to change the code.

oppiabot · 2021-12-29T15:43:44Z

Unassigning @vojtechjelinek since the review is done.

oppiabot · 2021-12-29T15:43:47Z

Hi @IamEzio, it looks like some changes were requested on this pull request by @vojtechjelinek. PTAL. Thanks!

IamEzio · 2021-12-29T19:17:02Z

@vojtechjelinek PTAL! Thanks

iamprayush

LGTM!

oppiabot · 2022-01-10T14:40:24Z

Unassigning @iamprayush since they have already approved the PR.

oppiabot · 2022-01-10T14:40:28Z

Hi @DubeySandeep, this PR is ready to be merged. Author of this PR does not have permissions to merge this PR. Before you merge it, please make sure that there are no pending comments that require action from the author's end. Thanks!

oppiabot · 2022-01-10T16:46:47Z

Hi @IamEzio. Due to recent changes in the "develop" branch, this PR now has a merge conflict. Please follow this link if you need help resolving the conflict, so that the PR can be merged. Thanks!

oppiabot · 2022-01-11T17:44:46Z

Hi @IamEzio, there is a new change in develop which needs to be in your PR. Please update your branch with the latest changes in develop. For instructions, refer to this link. Thanks!

DubeySandeep · 2022-01-11T19:37:56Z

@vojtechjelinek Can you please reply on this thread?

oppiabot · 2022-01-13T13:21:51Z

Hi @IamEzio. Due to recent changes in the "develop" branch, this PR now has a merge conflict. Please follow this link if you need help resolving the conflict, so that the PR can be merged. Thanks!

IamEzio · 2022-01-13T20:19:45Z

@DubeySandeep PTAL! I refactored the code which solved this failing backend test.

IamEzio added 2 commits December 28, 2021 22:24

changed deprecated-methods

3d227e7

changed deprecated methods

7a0b81a

IamEzio requested review from aks681, DubeySandeep, Hudda, iamprayush, nithusha21, seanlip, vojtechjelinek and a team as code owners December 28, 2021 18:37

IamEzio requested review from a team December 28, 2021 18:37

oppiabot bot added the REVIEWERS: Please add changelog label label Dec 28, 2021

oppiabot bot assigned seanlip and vojtechjelinek Dec 28, 2021

vojtechjelinek unassigned seanlip Dec 29, 2021

vojtechjelinek added PR CHANGELOG: Developer UX -- @DubeySandeep and removed REVIEWERS: Please add changelog label labels Dec 29, 2021

vojtechjelinek requested changes Dec 29, 2021

View reviewed changes

oppiabot bot unassigned vojtechjelinek Dec 29, 2021

oppiabot bot assigned IamEzio Dec 29, 2021

IamEzio and others added 3 commits December 29, 2021 23:01

Merge branch 'oppia:develop' into pylint-rule

f4657b1

restored changelog

b1bd920

restored changelog

5e49a47

oppiabot bot assigned vojtechjelinek and unassigned IamEzio Dec 29, 2021

Merge branch 'oppia:develop' into pylint-rule

746587d

iamprayush approved these changes Jan 10, 2022

View reviewed changes

oppiabot bot unassigned iamprayush Jan 10, 2022

oppiabot bot added the PR: LGTM label Jan 10, 2022

oppiabot bot added the PR: don't merge - HAS MERGE CONFLICTS label Jan 10, 2022

oppiabot bot assigned IamEzio Jan 10, 2022

Merge branch 'develop' into pylint-rule

0b92b90

oppiabot bot removed the PR: don't merge - HAS MERGE CONFLICTS label Jan 10, 2022

Merge branch 'oppia:develop' into pylint-rule

e61134e

DubeySandeep enabled auto-merge (squash) January 11, 2022 19:36

DubeySandeep disabled auto-merge January 11, 2022 19:37

DubeySandeep assigned vojtechjelinek and unassigned DubeySandeep Jan 11, 2022

oppiabot bot added the PR: don't merge - HAS MERGE CONFLICTS label Jan 13, 2022

Merge branch 'develop' into pylint-rule

e19c419

oppiabot bot removed the PR: don't merge - HAS MERGE CONFLICTS label Jan 13, 2022

refactored code

9346c62

IamEzio removed their assignment Jan 13, 2022

oppiabot bot assigned DubeySandeep Jan 13, 2022

DubeySandeep merged commit 2a115bb into oppia:develop Jan 14, 2022

IamEzio mentioned this pull request Jan 14, 2022

Fix failing mypy checks of core.domain.rights_domain_test #14649

Merged

4 tasks

IamEzio deleted the pylint-rule branch January 14, 2022 16:40

DiptoChakrabarty added a commit to DiptoChakrabarty/oppia that referenced this pull request Jan 16, 2022

replace deprecated method as per PR oppia#14506

5aaa984

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix part of #12912: Changes deprecated python methods #14506

Fix part of #12912: Changes deprecated python methods #14506

IamEzio commented Dec 28, 2021 •

edited

Loading

oppiabot bot commented Dec 28, 2021

vojtechjelinek left a comment

vojtechjelinek Dec 29, 2021

IamEzio Dec 29, 2021

vojtechjelinek Dec 30, 2021

IamEzio Dec 30, 2021

IamEzio Jan 3, 2022

IamEzio Jan 3, 2022

DubeySandeep Jan 11, 2022

vojtechjelinek Jan 11, 2022

oppiabot bot commented Dec 29, 2021

oppiabot bot commented Dec 29, 2021

IamEzio commented Dec 29, 2021

iamprayush left a comment

oppiabot bot commented Jan 10, 2022

oppiabot bot commented Jan 10, 2022

oppiabot bot commented Jan 10, 2022

oppiabot bot commented Jan 11, 2022

DubeySandeep commented Jan 11, 2022

oppiabot bot commented Jan 13, 2022

IamEzio commented Jan 13, 2022

		sig = inspect.signature(self._func)

		args_dict = sig.bind_partial(args, *kwargs)

Fix part of #12912: Changes deprecated python methods #14506

Fix part of #12912: Changes deprecated python methods #14506

Conversation

IamEzio commented Dec 28, 2021 • edited Loading

Overview

Essential Checklist

Proof that changes are correct

PR Pointers

oppiabot bot commented Dec 28, 2021

vojtechjelinek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oppiabot bot commented Dec 29, 2021

oppiabot bot commented Dec 29, 2021

IamEzio commented Dec 29, 2021

iamprayush left a comment

Choose a reason for hiding this comment

oppiabot bot commented Jan 10, 2022

oppiabot bot commented Jan 10, 2022

oppiabot bot commented Jan 10, 2022

oppiabot bot commented Jan 11, 2022

DubeySandeep commented Jan 11, 2022

oppiabot bot commented Jan 13, 2022

IamEzio commented Jan 13, 2022

IamEzio commented Dec 28, 2021 •

edited

Loading