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

Closes #70: create pyproject #71

Merged
merged 4 commits into from
Dec 23, 2022
Merged

Closes #70: create pyproject #71

merged 4 commits into from
Dec 23, 2022

Conversation

thodson-usgs
Copy link
Collaborator

Setting up a pyproject.toml, which I've never done before.
This should

  • acknowledge Jay
  • add a description to the PYPI page
  • add project urls
  • dynamically track versioning

@thodson-usgs
Copy link
Collaborator Author

This one might require two passes. One to review the pyroject.toml and one to understand why all these builds are failing.

Copy link
Contributor

@elbeejay elbeejay left a comment

Choose a reason for hiding this comment

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

@thodson-usgs I think the documentation build failure and the pyproject.toml are related. I've never written a .toml or done Python packaging this way, so I'm certainly not an expert, but it seems to be the modern way of doing it, so I'm glad to see it.

The documentation build, unlike the testing and linting one, actually attempts to install the dataretrieval package itself. This is where I think it is failing, around line 215 of the log file.

The command it is trying to run is:

pip install .

And the corresponding error seems to be:

Defaulting to user installation because normal site-packages is not writeable
Processing /home/runner/work/dataretrieval/dataretrieval
  Installing build dependencies: started
  Installing build dependencies: finished with status 'done'
  Getting requirements to build wheel: started
  Getting requirements to build wheel: finished with status 'error'
  error: subprocess-exited-with-error
  
  × Getting requirements to build wheel did not run successfully.
  │ exit code: 1
  ╰─> [14 lines of output]
      error: Multiple top-level packages discovered in a flat-layout: ['demos', 'dataretrieval'].
      
      To avoid accidental inclusion of unwanted files or directories,
      setuptools will not proceed with this build.
      
      If you are trying to create a single distribution with multiple packages
      on purpose, you should not rely on automatic discovery.
      Instead, consider the following options:
      
      1. set up custom discovery (`find` directive with `include` or `exclude`)
      2. use a `src-layout`
      3. explicitly set `py_modules` or `packages` with a list of names
      
      To find more information, look for "package discovery" on setuptools docs.
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
error: subprocess-exited-with-error

× Getting requirements to build wheel did not run successfully.
│ exit code: 1
╰─> See above for output.

My guess is that when the setuptools.setup() function is running, I think the function needs more information about which subdirectory is the package, it seems to be flagging 'demos' as a potential top-level module. I think per the setuptools documentation adding the following lines to the .toml file might work?

[tool.setuptools]
packages = ["dataretrieval", "dataretrieval.codes"]

Then just from my browsing about the .toml setup, I'm not sure if we will need to potentially require "wheels" or add a "shim" to setup.py but I think we want to continue supporting pip-installation, editable pip-installs, as well as the conda-installation. I know pip needs to be able to create wheels, and I think conda does too? so this might be something we'll need to keep an eye on until tests pass and then after merging and publishing the latest versions.

@elbeejay
Copy link
Contributor

As a side-note, the regular build tests are more likely to fail now because there are several recent additions that actually execute API queries to the web services rather than mocking the expected returns. The advantage was that we will know when services change as the tests will fail, but we suffer a bit with less reliable tests as the web services have to be online and returning data consistently for them all to pass.

Copy link
Contributor

@elbeejay elbeejay left a comment

Choose a reason for hiding this comment

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

Looks like all the builds now pass, so I'm good with merging it now.

@thodson-usgs
Copy link
Collaborator Author

[tool.setuptools]
packages = ["dataretrieval", "dataretrieval.codes"]

did the trick.

@elbeejay elbeejay merged commit bd1de33 into DOI-USGS:master Dec 23, 2022
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.

2 participants