Pylint interventions mpcontribs api#1857
Closed
evidencebp wants to merge 26 commits intomaterialsproject:masterfrom
Closed
Pylint interventions mpcontribs api#1857evidencebp wants to merge 26 commits intomaterialsproject:masterfrom
evidencebp wants to merge 26 commits intomaterialsproject:masterfrom
Conversation
Made a readable line shorter
Was a very long format line with 135 characters which wasn't readable. I reduced it to 107, higher than the recommended 100, while keeping the main part in the same line.
Was a very long format line with 135 characters which wasn't readable. I reduced it to 107, higher than the recommended 100, while keeping the main part in the same line.
Readable line in commented out code
…ocks The post_save class method had 6 nesting levels while pylint recommends not to have more than 5. I extracted the small and targeted update_columns_by_flat method, with 3 nesting levels.
…ildcard-import Wildcard imports make it harder to understand what is imported from where. Removing it is also a defensive programming act, lowering the probability of collisions due to future new imports or objects.
The methods get_string and from_string of the class MPFile had 14 branches each. Pylint recommends not to have more than 12 to reduce complexity, ease understanding and testing. I extracted from each method a small local method to reduce the number of branches and increase structuring.
Method run had 82 statements while pylint suggest not to have more than 50. I extracted some small local methods, adding structure and making the method shorter. Please note that the end of the method was not reachable (and still so) due to a break, which might be a bug. I added a TODO there to mark that.
The method add_structure of class MPFileCore had 17 branches while pylint recommends to have no more than 12. I extracted a small method _build_structure that builds the structure from the source while using many branches to handle the source possible types.
…sion.py too-many-statements Function run had 117 statements while pylint recommends not to have more than 50. The function was actually structured since it had different logics for different crystal structures. I extracted small unrelated functions for these logics.
The make function had 74 statement while pylint recommends to have at most 50. I extracted small methods handling parts of the function.
…on-caught Catching Exception instead of specific exceptions might hide unexpected ones. Function format_cell caught Exception. isnan does not raise exceptions https://docs.python.org/3/library/math.html#math.isnan str might lead to UnicodeEncodeError https://stackoverflow.com/questions/33556465/is-there-any-object-can-make-str-function-throw-error-or-exception-in-python
…caught Method to_backgrid_dict of class table catches Exception. Catching Exception instead of specific exceptions might hide unexpected ones. I changed to catching AttributeError, as in the StackOverflow post in the comment above.
Function get_specs had 19 branches while pylint suggest not to have more than 12. I extracted method for populating the data structures and handling the different method names.
…ission Method has_read_permission of class SwaggerView had 20 branches while pylint recommends to have no more than 12. I extracted method to reduce that, making the code more structured. This fixed also the too-many-return-statements and too-many-statements alerts in the same method.
* update dependencies for mpcontribs-api (ubuntu-latest/py3.10) * update dependencies for mpcontribs-api (ubuntu-latest/py3.11) * update dependencies for mpcontribs-client (ubuntu-latest/py3.10) * update dependencies for mpcontribs-client (ubuntu-latest/py3.11) * update dependencies for mpcontribs-kernel-gateway (ubuntu-latest/py3.10) * update dependencies for mpcontribs-kernel-gateway (ubuntu-latest/py3.11) * update dependencies for mpcontribs-portal (ubuntu-latest/py3.10) * update dependencies for mpcontribs-portal (ubuntu-latest/py3.11) --------- Co-authored-by: github-actions <github-actions@github.com>
…ception-caught" This reverts commit ae58518.
This reverts commit 188f43f.
…oo-long" This reverts commit 6b351fa.
…n__.py wildcard-import" This reverts commit f6dbc35.
This reverts commit 784a6c4.
This reverts commit 6e5ad88.
This reverts commit b56e230.
…e_submission.py too-many-statements" This reverts commit bed45b5.
4 tasks
evidencebp
added a commit
to evidencebp/pylint-intervention
that referenced
this pull request
Nov 19, 2024
| "status": "ERROR", "cid": cid, "count": count, "total": total, "exc": str(e) | ||
| } | ||
| return ret | ||
|
|
Collaborator
There was a problem hiding this comment.
Closing since this is quite a bit out of date and I can see spots like this where the code is clearly going to fail (ret is initialized as None but is later mutated to dict; this refactor also ignores that ret is already populated in the code block where _update_doc_notebook is called)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The pylint intervention PR with commits not related to mpcontribs-api reverted