testcontainers-python icon indicating copy to clipboard operation
testcontainers-python copied to clipboard

refactor: Migrate to the flat layout

Open kshramt opened this issue 1 year ago • 11 comments
trafficstars

If this PR is accepted, I intend to follow up with another PR to add py.typed to address https://github.com/testcontainers/testcontainers-python/issues/305 .

kshramt avatar Mar 20 '24 17:03 kshramt

(I am not sure why "5 workflows awaiting approval" is not shown.)

kshramt avatar Mar 22 '24 14:03 kshramt

I plan to rebase the PR after the review.

kshramt avatar Mar 27 '24 16:03 kshramt

marking as draft because i think we need a larger discussion before making this kind of change (for example, i am not convinced but i havent thought about it too much, maybe i am missing an argument for (or against))

alexanderankin avatar Mar 30 '24 23:03 alexanderankin

@kshramt I read through the referenced issue, but I don't get the context of why we would want a flat package hierarchy? Having all community packages in the module directory is quite clean, but if there are any strong arguments on why it shouldn't be that way I am eager to listen.

santi avatar Apr 04 '24 08:04 santi

@santi, thanks for the comment. I appreciate the neatness of the current structure but have observed some challenges, especially for those new to the unique packaging patterns. When I set out to integrate MyPy support, my initial expectation was straightforward: simply run touch py.typed and address type errors. However, figuring out how to properly include py.typed in the existing structure—and how to run MyPy and similar tools—wasn't as intuitive as I had hoped. This deviation from more conventional Python project structures required additional research and effort, which could be a hurdle for new contributors.

Additionally, specific adjustments, such as those detailed in https://github.com/testcontainers/testcontainers-python/commit/507e466a1fa9ac64c254ceb9ae0d57f6bfd8c89d#diff-50c86b7ed8ac2cf95bd48334961bf0530cdc77b5a56f852c5c61b89d735fd711R33 and https://github.com/testcontainers/testcontainers-python/commit/507e466a1fa9ac64c254ceb9ae0d57f6bfd8c89d#diff-50c86b7ed8ac2cf95bd48334961bf0530cdc77b5a56f852c5c61b89d735fd711R234, are rendered unnecessary with a transition to the flat layout. Migrating to this layout not only makes the project structure more intuitive for everyone involved but also minimizes maintenance overhead associated with tools' structural requirements.

I believe that adopting the more common flat layout could make the project more accessible and easier to maintain. I'm genuinely interested in your perspective on this issue. Do you have any specific concerns with the flat layout, or are there other solutions you believe might be more effective?

kshramt avatar Apr 06 '24 12:04 kshramt

the part of the project under core/ is the only part which truly needs to be maintainer friendly. the rest is a demonstration of what core/ can do. e.g., the segmentation is what allows a partial effort to type the package, instead of needing to do the whole thing at once.

alexanderankin avatar Apr 06 '24 13:04 alexanderankin

the segmentation is what allows a partial effort to type the package, instead of needing to do the whole thing at once.

I understand the rationale behind segmentation for facilitating partial typing efforts. However, it's worth noting that the flexibility for partial typing can still be maintained within the flat layout. As outlined in https://peps.python.org/pep-0561/#packaging-type-information , package maintainers aiming to support type checking must include a py.typed marker file in their packages to indicate support for typing. Importantly, this marker file's presence applies recursively, meaning if a top-level package is marked with py.typed, all its sub-packages are also considered to support type checking.

If I interpret correctly, this implies that by placing a py.typed file within testcontainers/core, and then running mypy testcontainers/core, we effectively achieve partial typing. This approach aligns with PEP 561's guidelines and ensures that we can selectively type parts of the project without necessitating a complete overhaul at once. This strategy should allow us to enjoy the benefits of the flat layout while still accommodating partial typing efforts as needed.

kshramt avatar Apr 07 '24 05:04 kshramt

For distribution, according to https://github.com/python/typing/discussions/1429, a partially annotated package with the root py.typed file also appears to be acceptable.

kshramt avatar Apr 07 '24 05:04 kshramt

Personally, I would argue that everything should be typed and failing to do so is a bug. At least in my books, a untyped library reflects badly on how I would judge the library nowadays (looking at confluent kafka...).

jankatins avatar Apr 26 '24 09:04 jankatins

I am with @kshramt on this. But I would prefer a src layout instead of flat layout.

Doing so it more clear what is actually packages and prevents some problems testing the wrong package....

CarliJoy avatar Jul 19 '24 16:07 CarliJoy

Hey all!

Since I wrote the refactor to the current core and modules, I can chime in a bit.

There is a very simple reason why it is core and modules -> it is because all major testcontainers language flavors use these. I don't think I'd want to deviate from that as it allows the TC mainline documentation to converge and easily match support across languages (automation can be built for it, and issues tracked across language flavours)

I wish we had better automation and support from the TC staff (I'm only a volunteer myself like @alexanderankin) but this is what we've got for now.

totallyzen avatar Aug 02 '24 09:08 totallyzen