Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Fixing the Formatting of the bitCountByte Variable: pymysqlreplication/bitmap.py #540

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

Open
groom2hub wants to merge 2 commits into julien-duponchelle:main
base: main
Choose a base branch
Loading
from groom2hub:enhancement/refomating-bitcountbyte-groom2hub

Conversation

@groom2hub
Copy link
Contributor

@groom2hub groom2hub commented Oct 15, 2023
edited
Loading

I've improved readability by breaking the bitCountByte variable into lines, with 16 items per line.

close #539

Copy link
Collaborator

@sean-k1 sean-k1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comment

2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6,
3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7,
3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7,
4, 5, 5, 6, 5, 6, 6, 7, 5, 6, 6, 7, 6, 7, 7, 8,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to write #fmt on
Avoid black convention
And can you change title kor to eng?

Copy link
Contributor Author

I've improved readability by breaking the bitCountByte variable into lines, with 16 items per line.

@groom2hub groom2hub changed the title (削除) bitCountByte 변수 포매팅 고치기: pymysqlreplication/bitmap.py (削除ここまで) (追記) Fixing the Formatting of the bitCountByte Variable: pymysqlreplication/bitmap.py (追記ここまで) Oct 15, 2023
- black does not provide a way to ignore by convention
- ignore by specifying file name
Copy link
Contributor

if you want to do this you have to fix this code like this

# fmt: off
bitCountInByte = [
 0, 1, 1, 2, 1, 2, 2, 3, 1, 2, 2, 3, 2, 3, 3, 4, 
 1, 2, 2, 3, 2, 3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5, 
 1, 2, 2, 3, 2, 3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5, 
 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6, 
 1, 2, 2, 3, 2, 3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5,
 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6,
 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6, 
 3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7,
 1, 2, 2, 3, 2, 3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5,
 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6,
 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6,
 3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7,
 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6,
 3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7,
 3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7,
 4, 5, 5, 6, 5, 6, 6, 7, 5, 6, 6, 7, 6, 7, 7, 8,
]
# fmt: on

and how about changing variable name instead of applying formatting which I have done eariler

https://github.com/julien-duponchelle/python-mysql-replication/pull/518/files

Copy link
Contributor

why-arong commented Oct 15, 2023
edited
Loading

I believe it would be beneficial to address all modifications related to 'bitmap.py' in this pull request at once.

It would be a good idea to convert variables and functions written in camelCase, such as 'bitCountInByte,' to snake_case to adhere to the PEP 8 convention.

sean-k1 and dongwook-chan reacted with thumbs up emoji


ruff pymysqlreplication
black pymysqlreplication --check
black pymysqlreplication --exclude pymysqlreplication/bitmap.py --check
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not exclude all bitmap.py

apply # fmt: on is more better

why-arong reacted with thumbs up emoji
Copy link
Contributor

@why-arong why-arong Oct 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be helpful if you refer to this document

It doesn’t reformat lines that end with # fmt: skip or blocks that start with # fmt: off and end with # fmt: on. # fmt: on/off must be on the same level of indentation and in the same block, meaning no unindents beyond the initial indentation level between them.

Copy link
Collaborator

if you want to do this you have to fix this code like this

# fmt: off
bitCountInByte = [
 0, 1, 1, 2, 1, 2, 2, 3, 1, 2, 2, 3, 2, 3, 3, 4, 
 1, 2, 2, 3, 2, 3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5, 
 1, 2, 2, 3, 2, 3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5, 
 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6, 
 1, 2, 2, 3, 2, 3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5,
 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6,
 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6, 
 3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7,
 1, 2, 2, 3, 2, 3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5,
 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6,
 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6,
 3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7,
 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6,
 3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7,
 3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7,
 4, 5, 5, 6, 5, 6, 6, 7, 5, 6, 6, 7, 6, 7, 7, 8,
]
# fmt: on

and how about changing variable name instead of applying formatting which I have done eariler

https://github.com/julien-duponchelle/python-mysql-replication/pull/518/files

I agree that this is a better solution, because it leaves the rest of codes subject to the lint test.
But the fact that the solution is to put comments seems a bit too intrusive for me.
Because these are not a real comment and adding fake comments just for the sake of the lint test?
I'm not sure. I don't think the best solution should lie in the code section (*.py) at all.
@soulee-dev Any ideas?

Copy link
Contributor

if you want to do this you have to fix this code like this

# fmt: off
bitCountInByte = [
 0, 1, 1, 2, 1, 2, 2, 3, 1, 2, 2, 3, 2, 3, 3, 4, 
 1, 2, 2, 3, 2, 3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5, 
 1, 2, 2, 3, 2, 3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5, 
 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6, 
 1, 2, 2, 3, 2, 3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5,
 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6,
 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6, 
 3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7,
 1, 2, 2, 3, 2, 3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5,
 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6,
 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6,
 3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7,
 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6,
 3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7,
 3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7,
 4, 5, 5, 6, 5, 6, 6, 7, 5, 6, 6, 7, 6, 7, 7, 8,
]
# fmt: on

and how about changing variable name instead of applying formatting which I have done eariler
https://github.com/julien-duponchelle/python-mysql-replication/pull/518/files

I agree that this is a better solution, because it leaves the rest of codes subject to the lint test. But the fact that the solution is to put comments seems a bit too intrusive for me. Because these are not a real comment and adding fake comments just for the sake of the lint test? I'm not sure. I don't think the best solution should lie in the code section (*.py) at all. @soulee-dev Any ideas?

maybe if we seperate bitCountByte into another independent file and exclude from linting. It can be a solution

dongwook-chan reacted with thumbs up emoji

Copy link
Collaborator

if you want to do this you have to fix this code like this

# fmt: off
bitCountInByte = [
 0, 1, 1, 2, 1, 2, 2, 3, 1, 2, 2, 3, 2, 3, 3, 4, 
 1, 2, 2, 3, 2, 3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5, 
 1, 2, 2, 3, 2, 3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5, 
 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6, 
 1, 2, 2, 3, 2, 3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5,
 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6,
 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6, 
 3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7,
 1, 2, 2, 3, 2, 3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5,
 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6,
 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6,
 3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7,
 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6,
 3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7,
 3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7,
 4, 5, 5, 6, 5, 6, 6, 7, 5, 6, 6, 7, 6, 7, 7, 8,
]
# fmt: on

and how about changing variable name instead of applying formatting which I have done eariler
https://github.com/julien-duponchelle/python-mysql-replication/pull/518/files

I agree that this is a better solution, because it leaves the rest of codes subject to the lint test. But the fact that the solution is to put comments seems a bit too intrusive for me. Because these are not a real comment and adding fake comments just for the sake of the lint test? I'm not sure. I don't think the best solution should lie in the code section (*.py) at all. @soulee-dev Any ideas?

maybe if we seperate bitCountByte into another independent file and exclude from linting. It can be a solution

I looooovvveee your solution.
We have a handful of enumerators and variables misplaced in some random files.
We can gather and categorize them and create new files and make them exceptions.

Copy link
Contributor

I

can we find out varables and enums together?

Copy link
Collaborator

@mirageoasis

This is an alternative to a lookup list.

def bit_count(n):
 return bin(n).count('1')

This is more readable and I would say more Pythonic too (but not in a strict sense).
However, since python-mysql-replication emulates implementations in mysql-server and look-up table is such common in any RDBMS implementations, I think look-up table is a great solution.

I'll do some research on other projects that emulate RDBMS and see how they implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sean-k1 sean-k1 sean-k1 left review comments

+1 more reviewer

@why-arong why-arong why-arong left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Apply f-string formatting to bitCountByte: pymysqlreplication/bitmap.py

AltStyle によって変換されたページ (->オリジナル) /