Skip to content
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

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

groom2hub
Copy link
Contributor

@groom2hub groom2hub commented Oct 15, 2023

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?

@groom2hub
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
@mirageoasis
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

@why-arong
Copy link
Contributor

why-arong commented Oct 15, 2023

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.

@@ -4,4 +4,4 @@ set -e
set -x

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

Copy link
Contributor

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.

@dongwook-chan
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?

@mirageoasis
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
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.

@mirageoasis
Copy link
Contributor

I

can we find out varables and enums together?

@dongwook-chan
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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