Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to implicit (PEP 420) namespace. #8

Merged
merged 4 commits into from
Mar 22, 2024
Merged

Switch to implicit (PEP 420) namespace. #8

merged 4 commits into from
Mar 22, 2024

Conversation

icemac
Copy link
Member

@icemac icemac commented Jan 24, 2024

Open issues:

  • No tests are run

@icemac icemac self-assigned this Jan 24, 2024
@dataflake
Copy link
Member

Those small changes are all that's required?

@gforcada
Copy link
Member

While testing it at work, I noticed the same, tests do not run.

Not sure if it is connected with zope.testrunner 🤔

@dataflake dataflake changed the title Switch to implicit (PEP 440) namespace. Switch to implicit (PEP 420) namespace. Jan 24, 2024
@dataflake
Copy link
Member

The problem is indeed zope.testrunner, specifically the following lines linked below. It's not compatible with implicit namespace packages because it will remove those folders from the test search path that do not contain an __init__.py, so in this particular case it will give up in the folder src/megrok because that does not contain the file anymore. Commenting out those lines will lead to a noticeable slowdown finding tests, but it does find and run them.

https://github.com/zopefoundation/zope.testrunner/blob/4f9048acb7f54677e6e6890b8491e069d136a2d6/src/zope/testrunner/find.py#L295-L299

@dataflake
Copy link
Member

After checking again it looks like the perceived slowdown doesn't really exist. So the fix is in zope.testrunner's find.find_test_files_ function. Commenting out the offending lines breaks a number of zope.testrunner unit/doc tests, though. Unfortunately there's nothing in the local and global namespaces at that point in the code that could identify the folder named after the shared namespace so that it could be excluded from that shortcut, just like the folder where the search starts is excluded.

@icemac
Copy link
Member Author

icemac commented Jan 25, 2024

@dataflake Thank you for digging into the issue. I created a ticket for it: zopefoundation/zope.testrunner#160

@icemac
Copy link
Member Author

icemac commented Jan 25, 2024

@dataflake wrote:

Those small changes are all that's required?

Yes, I think so. But it has be done for all packages in a namespace at the same time. I chose this package to try out implicit namespaces because it is the only one in its namespace.

See also https://packaging.python.org/en/latest/guides/packaging-namespace-packages/

@dataflake
Copy link
Member

In an experiment on a different package I had to add the following to the setup call to make it work, otherwise the release-check step could not build the package:

      package_dir={'': 'src'},

@icemac
Copy link
Member Author

icemac commented Jan 26, 2024

In an experiment on a different package I had to add the following to the setup call to make it work, otherwise the release-check step could not build the package:

      package_dir={'': 'src'},

This is what we usually do, so it is already in all setup.py files. It is also necessary for the pkg_resouces namespaces approach.

@icemac icemac marked this pull request as ready for review March 21, 2024 07:43
Copy link
Member

@dataflake dataflake left a comment

Choose a reason for hiding this comment

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

I've done it the same way for my minor packages in the dataflake namespace and it worked out just fine. There's just a few so I changed them all at the same time and published releases. I am scared of doing this in the zope or Products namespace. That all needs to be closely coordinated for hundreds of packages.

@icemac
Copy link
Member Author

icemac commented Mar 22, 2024

@dataflake It will be a major update for all the packages. So not updating is still a possibility. And yes, it is a mayor effort, but it is nice to see that it is possible.

@icemac icemac merged commit ca4edbb into master Mar 22, 2024
13 checks passed
@icemac icemac deleted the pep440 branch March 22, 2024 08:11
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.

None yet

3 participants