Skip to content

Conversation

@mxyng
Copy link
Collaborator

@mxyng mxyng commented Feb 14, 2025

this change decouples project management from virtualenv management such that developers can use their favorite tools instead of being forced into poetry.

poetry 2.0 adds pep 621 support so updating pyproject.toml to be pep 621 compliant requires many of the changes necessary to use hatch while also having to manually maintain separate dependency groups for dev tools such as pytest and ruff

fix some linting issues found by newly enabled linters

  • C416 - unnecessary list comprehension
  • FBT001 - positional boolean function arguments
  • FLY002 - replace '://'.join(scheme, hostport) with f'{scheme}://{hostport}'
  • PIE790 - unnecessary pass
  • PIE810 - replace multiple startswidth with startswith(tuple)
  • SIM105 - use contextlib.suppress instead of try except pass
  • SIM101 - replace multiple isinstance with isinstance(tuple)
  • SIM108 - use ternary operator instead of if else

@mxyng mxyng force-pushed the mxyng/hatch branch 3 times, most recently from bf5d8e9 to 3a8fcb4 Compare February 14, 2025 09:23
'foo'
"""
return self[key] if key in self else default
return getattr(self, key) if hasattr(self, key) else default
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

linter suggests self.get(k, default) but that'll call itself

return parsed_docstring

key = hash(doc_string)
key = str(hash(doc_string))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

mypy complains about a type mismatch for key since hash(...) returns an int but may be set to a 'args', i.e. string, later

messages: Optional[Sequence[Union[Mapping[str, Any], Message]]] = None,
*,
stream: Literal[True] = True,
stream: Literal[False] = False,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This overload was incorrect since stream=True is also overloaded below

async def create_blob(self, path: Union[str, Path]) -> str:
sha256sum = sha256()
with open(path, 'rb') as r:
async with await anyio.open_file(path, 'rb') as r:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

should not use a blocking open in an async function

- run: pipx install poetry
- uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}
Copy link
Member

Choose a reason for hiding this comment

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

Any reason for removing the matrix tests?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is replaced by hatch test --all which tests all configured python versions. I'm still debating whether this is better than matrix tests

Copy link
Member

Choose a reason for hiding this comment

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

Seems like we can setup a matrix if needed in the pyproject.toml if needed: https://hatch.pypa.io/latest/tutorials/testing/overview/#all-environments

I think it makes sense to just define the versions we want tested in the toml

Copy link
Collaborator Author

@mxyng mxyng Feb 20, 2025

Choose a reason for hiding this comment

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

That's already implemented and runs with --all option to hatch test however it is slightly slower since the Python versions are sequential rather than parallel, ~1m vs. ~35s

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No that option, which is also set, parallelizes tests in a pytest run, not between python versions

@mxyng mxyng merged commit 5ae5f81 into main May 6, 2025
2 checks passed
@mxyng mxyng deleted the mxyng/hatch branch May 6, 2025 18:03
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