Skip to content

chore(module): restructure the main pysolr module - #573

Merged
acdha merged 1 commit into
django-haystack:masterfrom
DhavalGojiya:chore/refactor-module-structure
Sep 8, 2026
Merged

chore(module): restructure the main pysolr module#573
acdha merged 1 commit into
django-haystack:masterfrom
DhavalGojiya:chore/refactor-module-structure

Conversation

@DhavalGojiya

Copy link
Copy Markdown
Contributor

Description

  • Restructured the main pysolr module
  • Grouped constants, functions, and classes together

@DhavalGojiya
DhavalGojiya force-pushed the chore/refactor-module-structure branch from 1dbdec1 to 5c53dda Compare June 5, 2026 10:16
@DhavalGojiya

Copy link
Copy Markdown
Contributor Author

Your review plz : @cclauss

@cclauss

cclauss commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Why not use str.maketrans() and str.translate()?

@DhavalGojiya
DhavalGojiya force-pushed the chore/refactor-module-structure branch from 5c53dda to f8b84ea Compare June 22, 2026 08:36
@DhavalGojiya

Copy link
Copy Markdown
Contributor Author

Why not use str.maketrans() and str.translate()?

Done :)

spent some time learning about these methods.
Thanks @cclauss. Every day, I am learning something new from you. 😅

@DhavalGojiya

Copy link
Copy Markdown
Contributor Author

Your review plz: @acdha

Comment thread pysolr.py Outdated
Comment thread pysolr.py Outdated
@DhavalGojiya
DhavalGojiya force-pushed the chore/refactor-module-structure branch from f8b84ea to 982ce84 Compare August 10, 2026 15:08
@DhavalGojiya
DhavalGojiya requested a review from acdha August 10, 2026 15:10
Comment thread pysolr.py Outdated
@DhavalGojiya
DhavalGojiya force-pushed the chore/refactor-module-structure branch from 982ce84 to cbd5436 Compare August 24, 2026 09:50
@DhavalGojiya
DhavalGojiya requested a review from acdha August 24, 2026 09:50
Comment thread pysolr.py Outdated
are stripped, except for tab, newline and carriage return, which are
valid whitespace.
"""
sanitized_chars_table = str.maketrans(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This should be up at the module level so it's not run on every call.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

if we want to avoid leaking a temporary variable, the easiest way to do that would be to put it into the only place it's used:

@acdha acdha Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sorry if there was any confusion, my example was at the module level combining the _CONTROL_CHARS definition and maketrans result into a single module-level declaration.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done :)

@DhavalGojiya
DhavalGojiya force-pushed the chore/refactor-module-structure branch from cbd5436 to 08e7bb5 Compare September 8, 2026 14:22
@DhavalGojiya
DhavalGojiya requested a review from acdha September 8, 2026 14:22
@acdha
acdha merged commit 808bb49 into django-haystack:master Sep 8, 2026
13 checks passed
@DhavalGojiya
DhavalGojiya deleted the chore/refactor-module-structure branch September 9, 2026 07:09
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.

3 participants