-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pylint interventions mpcontribs api #1857
Open
evidencebp
wants to merge
26
commits into
materialsproject:master
Choose a base branch
from
evidencebp:pylint_interventions_mpcontribs-api
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Pylint interventions mpcontribs api #1857
evidencebp
wants to merge
26
commits into
materialsproject:master
from
evidencebp:pylint_interventions_mpcontribs-api
Conversation
This file contains 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
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 <[email protected]>
…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
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