Skip to content

Fixes#29

Closed
KOLANICH wants to merge 6 commits intopolysquare:masterfrom
prebuilder:fixes
Closed

Fixes#29
KOLANICH wants to merge 6 commits intopolysquare:masterfrom
prebuilder:fixes

Conversation

@KOLANICH
Copy link
Copy Markdown

No description provided.

# enums of different types. Since we could be analyzing
# quite a lot of code, performance is more important than safety here.
class WordType(object): # suppress(R0903,too-few-public-methods)
class WordType(IntEnum): # suppress(R0903,too-few-public-methods)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Have a look at the comment above - did you performance test this code?

Copy link
Copy Markdown
Author

@KOLANICH KOLANICH Mar 10, 2020

Choose a reason for hiding this comment

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

No, I didn't, but I expect it to be a bit worse, since IntEnum members are members of a real class in python, not dumb numbers. Though I think convenience when debugging (IntEnum shows names when repred) is more important here than performance. IDK if using just a class of strs will give performance.

@smspillaz
Copy link
Copy Markdown
Member

(Sorry for the slow response, I missed these changes in the barrage of email that I get every day)

In general I think a lot of these fixes depart with my own preferred (and familiar to me) style of maintaining python packages. However, as you can probably tell, I'm not actively maintaining this code anymore. So you're more than welcome to fork it and keep maintaining it. If you want to also upload to the pypi package, I can figure out a way to make you a maintainer.

@KOLANICH
Copy link
Copy Markdown
Author

KOLANICH commented Mar 10, 2020

In general I think a lot of these fixes depart with my own preferred (and familiar to me) style of maintaining python packages.

Please name explicitly what exactly you dislike. .editorconfig is needed because not everyone has the default settings of an editor being the ones that PEP 8 prescribes. setup.cfg is needed because using it is just better than storing them in setup.py because unlike setup.py from setup.cfg one can extract the info without interpreting untrusted code. I also can split this PR into three or four, if it makes any sense.

However, as you can probably tell, I'm not actively maintaining this code anymore. So you're more than welcome to fork it and keep maintaining it. If you want to also upload to the pypi package, I can figure out a way to make you a maintainer.

No, thanks. I still maintain too many packages (not in pypi though) and the count of them will likely increase, so I prefer not to receive others' packages.

@KOLANICH
Copy link
Copy Markdown
Author

Splitted into #32, #33, #34, #35

@KOLANICH KOLANICH closed this Mar 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants