Prepare package for the final release#17
Merged
Conversation
* Update docsrtrings * Add version flag to CLI * Unify imports in across test suites
Reviewer's GuideThis PR prepares the package for its final release by enriching documentation and metadata, adding CLI version and help flags, unifying imports in test suites, adjusting default configuration, and bumping the project version. Class diagram for updated CLI argument parsing and Randname coreclassDiagram
class Randname {
+VALID_SEX_OPTIONS
+randfirst()
+randlast()
+randfull()
}
class argparse_ArgumentParser {
+add_argument()
+parse_args()
+print_help()
}
class parse_args {
+args: Sequence[str] | None
+returns: tuple[argparse.Namespace, argparse.ArgumentParser]
}
Randname <.. parse_args
parse_args --> argparse_ArgumentParser : uses
parse_args --> Randname : uses VALID_SEX_OPTIONS
Class diagram for updated config module singleton loggerclassDiagram
class set_logger {
+level_name: str
+returns: logging.Logger
}
class logger {
<<singleton>>
logging_level: error
}
set_logger --> logger : creates
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Double-check that changing title to "rname" doesn’t break version resolution since version is loaded via version(title).
- parse_args now returns (args, parser) instead of args alone—verify any external callers depending on the old signature remain compatible.
- Removing MANIFEST.in may exclude non-code assets (e.g. name databases) from the package—ensure all required data files are still included in the distribution.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Double-check that changing __title__ to "rname" doesn’t break version resolution since __version__ is loaded via version(__title__).
- parse_args now returns (args, parser) instead of args alone—verify any external callers depending on the old signature remain compatible.
- Removing MANIFEST.in may exclude non-code assets (e.g. name databases) from the package—ensure all required data files are still included in the distribution.
## Individual Comments
### Comment 1
<location> `src/randname/__init__.py:52-53` </location>
<code_context>
)
-__title__ = "randname"
+__title__ = "rname"
+__name__ = "randname"
__version__ = version(__title__)
__author__ = "Adam Walkiewicz"
</code_context>
<issue_to_address>
**suggestion:** Inconsistent naming between __title__ and __name__ may cause confusion.
__title__ is set to "rname" while __name__ is "randname". Please clarify the distinction or align these values to avoid ambiguity.
</issue_to_address>
### Comment 2
<location> `tests/test_randnames_es.py:19` </location>
<code_context>
@classmethod
def tearDownClass(cls):
- core._inst.database = core.Randname.PATH_TO_DATABASE
+ randname.core._inst.database = randname.core.Randname.PATH_TO_DATABASE
def setUp(self):
</code_context>
<issue_to_address>
**question (testing):** Question about test isolation and database state reset.
If tests can alter the database state, consider using a temporary database or mocking to ensure proper isolation and prevent tests from affecting each other.
</issue_to_address>
### Comment 3
<location> `tests/test_randnames_es.py:36-39` </location>
<code_context>
def test_last_name_year(self):
year = random.choice(self.last_names_year_range)
- result = core.randlast(country=self.country, year=year)
+ result = randname.core.randlast(country=self.country, year=year)
self.assertIsInstance(result, str)
</code_context>
<issue_to_address>
**suggestion (testing):** Suggestion to add assertions for output format and value constraints.
Add assertions to verify the returned name is non-empty and matches expected patterns, such as containing no digits and having a reasonable length.
```suggestion
def test_last_name_year(self):
year = random.choice(self.last_names_year_range)
result = randname.core.randlast(country=self.country, year=year)
self.assertIsInstance(result, str)
self.assertTrue(result, "Returned last name should not be empty")
self.assertTrue(result.isalpha(), "Last name should contain only alphabetic characters")
self.assertGreaterEqual(len(result), 2, "Last name should be at least 2 characters long")
self.assertLess(len(result), 50, "Last name should be less than 50 characters long")
```
</issue_to_address>
### Comment 4
<location> `tests/test_randnames_pl.py:36-39` </location>
<code_context>
def test_last_name_year(self):
year = random.choice(self.last_names_year_range)
- result = core.randlast(country=self.country, year=year)
+ result = randname.core.randlast(country=self.country, year=year)
self.assertIsInstance(result, str)
</code_context>
<issue_to_address>
**suggestion (testing):** Suggestion to add assertions for output format and value constraints.
Add assertions to verify the returned name is non-empty and conforms to expected patterns.
```suggestion
def test_last_name_year(self):
year = random.choice(self.last_names_year_range)
result = randname.core.randlast(country=self.country, year=year)
self.assertIsInstance(result, str)
self.assertTrue(result, "Returned last name should not be empty")
self.assertTrue(result.isalpha(), "Returned last name should contain only alphabetic characters")
self.assertGreaterEqual(len(result), 2, "Returned last name should be at least 2 characters long")
self.assertLessEqual(len(result), 30, "Returned last name should be at most 30 characters long")
```
</issue_to_address>
### Comment 5
<location> `tests/test_randnames_es.py:32-34` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid loops in tests. ([`no-loop-in-tests`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/no-loop-in-tests))
<details><summary>Explanation</summary>Avoid complex code, like loops, in test functions.
Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
* loops
* conditionals
Some ways to fix this:
* Use parametrized tests to get rid of the loop.
* Move the complex logic into helpers.
* Move the complex part into pytest fixtures.
> Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / [Don't Put Logic in Tests](https://abseil.io/resources/swe-book/html/ch12.html#donapostrophet_put_logic_in_tests)
</details>
</issue_to_address>
### Comment 6
<location> `tests/test_randnames_es.py:65-67` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid loops in tests. ([`no-loop-in-tests`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/no-loop-in-tests))
<details><summary>Explanation</summary>Avoid complex code, like loops, in test functions.
Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
* loops
* conditionals
Some ways to fix this:
* Use parametrized tests to get rid of the loop.
* Move the complex logic into helpers.
* Move the complex part into pytest fixtures.
> Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / [Don't Put Logic in Tests](https://abseil.io/resources/swe-book/html/ch12.html#donapostrophet_put_logic_in_tests)
</details>
</issue_to_address>
### Comment 7
<location> `tests/test_randnames_es.py:98-100` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid loops in tests. ([`no-loop-in-tests`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/no-loop-in-tests))
<details><summary>Explanation</summary>Avoid complex code, like loops, in test functions.
Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
* loops
* conditionals
Some ways to fix this:
* Use parametrized tests to get rid of the loop.
* Move the complex logic into helpers.
* Move the complex part into pytest fixtures.
> Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / [Don't Put Logic in Tests](https://abseil.io/resources/swe-book/html/ch12.html#donapostrophet_put_logic_in_tests)
</details>
</issue_to_address>
### Comment 8
<location> `tests/test_randnames_es.py:102-104` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid loops in tests. ([`no-loop-in-tests`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/no-loop-in-tests))
<details><summary>Explanation</summary>Avoid complex code, like loops, in test functions.
Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
* loops
* conditionals
Some ways to fix this:
* Use parametrized tests to get rid of the loop.
* Move the complex logic into helpers.
* Move the complex part into pytest fixtures.
> Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / [Don't Put Logic in Tests](https://abseil.io/resources/swe-book/html/ch12.html#donapostrophet_put_logic_in_tests)
</details>
</issue_to_address>
### Comment 9
<location> `tests/test_randnames_pl.py:32-34` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid loops in tests. ([`no-loop-in-tests`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/no-loop-in-tests))
<details><summary>Explanation</summary>Avoid complex code, like loops, in test functions.
Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
* loops
* conditionals
Some ways to fix this:
* Use parametrized tests to get rid of the loop.
* Move the complex logic into helpers.
* Move the complex part into pytest fixtures.
> Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / [Don't Put Logic in Tests](https://abseil.io/resources/swe-book/html/ch12.html#donapostrophet_put_logic_in_tests)
</details>
</issue_to_address>
### Comment 10
<location> `tests/test_randnames_pl.py:65-67` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid loops in tests. ([`no-loop-in-tests`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/no-loop-in-tests))
<details><summary>Explanation</summary>Avoid complex code, like loops, in test functions.
Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
* loops
* conditionals
Some ways to fix this:
* Use parametrized tests to get rid of the loop.
* Move the complex logic into helpers.
* Move the complex part into pytest fixtures.
> Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / [Don't Put Logic in Tests](https://abseil.io/resources/swe-book/html/ch12.html#donapostrophet_put_logic_in_tests)
</details>
</issue_to_address>
### Comment 11
<location> `tests/test_randnames_pl.py:98-100` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid loops in tests. ([`no-loop-in-tests`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/no-loop-in-tests))
<details><summary>Explanation</summary>Avoid complex code, like loops, in test functions.
Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
* loops
* conditionals
Some ways to fix this:
* Use parametrized tests to get rid of the loop.
* Move the complex logic into helpers.
* Move the complex part into pytest fixtures.
> Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / [Don't Put Logic in Tests](https://abseil.io/resources/swe-book/html/ch12.html#donapostrophet_put_logic_in_tests)
</details>
</issue_to_address>
### Comment 12
<location> `tests/test_randnames_pl.py:102-104` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid loops in tests. ([`no-loop-in-tests`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/no-loop-in-tests))
<details><summary>Explanation</summary>Avoid complex code, like loops, in test functions.
Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
* loops
* conditionals
Some ways to fix this:
* Use parametrized tests to get rid of the loop.
* Move the complex logic into helpers.
* Move the complex part into pytest fixtures.
> Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / [Don't Put Logic in Tests](https://abseil.io/resources/swe-book/html/ch12.html#donapostrophet_put_logic_in_tests)
</details>
</issue_to_address>
### Comment 13
<location> `src/randname/__init__.py:53` </location>
<code_context>
"""Randname - A random name generator library.
This package provides functionality for generating random names from various
countries. It includes utilities for generating first names, last names, and
full names, with support for multiple countries and customizable options.
The package offers a simple API for generating random names that can be used in
testing, data generation, or any application requiring random name generation.
!! warning
This package uses pseudo-random generators from Python standard library.
This package should not be used for security purposes.
The base package contains limited dataset of names. It is easy to create
a collision.
Attributes:
__title__ (str): The title of the package.
__name__ (str): The name of the package.
__version__ (str): The current version of the package.
__author__ (str): The author of the package.
__license__ (str): The license under which the package is distributed.
Examples:
>>> import randname
>>> randname.randfirst()
'John'
>>> randname.randlast()
'Smith'
>>> randname.randfull()
'Jane Doe'
>>> randname.available_countries()
['PL', 'US', 'ES', ...]
Modules:
config: Configuration for logging.
core: Core functionality for generating random names.
database: Database handling for name data.
error: Custom exceptions for the randname library.
"""
from importlib.metadata import version
from randname.core import (
available_countries,
randfirst,
randfull,
randlast,
show_data,
)
__title__ = "rname"
__name__ = "randname"
__version__ = version(__title__)
__author__ = "Adam Walkiewicz"
__license__ = "MIT"
__all__ = [
"available_countries",
"randfirst",
"randfull",
"randlast",
"show_data",
]
</code_context>
<issue_to_address>
**issue (code-quality):** Don't assign to builtin variable `\_\_name\_\_` ([`avoid-builtin-shadow`](https://docs.sourcery.ai/Reference/Default-Rules/comments/avoid-builtin-shadow/))
<br/><details><summary>Explanation</summary>Python has a number of `builtin` variables: functions and constants that
form a part of the language, such as `list`, `getattr`, and `type`
(See https://docs.python.org/3/library/functions.html).
It is valid, in the language, to re-bind such variables:
```python
list = [1, 2, 3]
```
However, this is considered poor practice.
- It will confuse other developers.
- It will confuse syntax highlighters and linters.
- It means you can no longer use that builtin for its original purpose.
How can you solve this?
Rename the variable something more specific, such as `integers`.
In a pinch, `my_list` and similar names are colloquially-recognized
placeholders.</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+20
to
+53
| __title__ = "rname" | ||
| __name__ = "randname" |
There was a problem hiding this comment.
suggestion: Inconsistent naming between title and name may cause confusion.
title is set to "rname" while name is "randname". Please clarify the distinction or align these values to avoid ambiguity.
This file contains hidden or 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
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 by Sourcery
Prepare package for final release by updating documentation, standardizing test imports, bumping package metadata, adding a CLI version flag, adjusting logging defaults, and cleaning up packaging artifacts.
New Features:
Enhancements:
Documentation:
Tests:
Chores: