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

MAINT: Fix tests with updated upstream #766

Merged
merged 10 commits into from Oct 10, 2023

Conversation

choldgraf
Copy link
Member

This updates our regression tests so that they pass. Will look at the diff and see if anything meaningful has changed. If not then I'll just merge so that we can inspect the other PRs more effectively.

@choldgraf
Copy link
Member Author

This all looks to be cosmetic and based on changes in the pydata theme, so let's tackle any UI changes in subsequent PRs and merge this one if tests are happy.

@choldgraf
Copy link
Member Author

@agoose77 any idea what the broken RTD error is? Here's the relevant bit, it seems to be choking when it tries to install pandas:

  × python setup.py egg_info did not run successfully.
  │ exit code: 1
  ╰─> [15 lines of output]
      /tmp/pip-install-6ricahb1/pandas_c91e810c40ee4cf7a965f5b8bf5a9150/setup.py:12: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
        import pkg_resources
      /home/docs/checkouts/readthedocs.org/user_builds/sphinx-book-theme/envs/766/lib/python3.7/site-packages/setuptools/__init__.py:84: _DeprecatedInstaller: setuptools.installer and fetch_build_eggs are deprecated.
      !!
      
              ********************************************************************************
              Requirements should be satisfied by a PEP 517 installer.
              If you are using pip, you can try `pip install --use-pep517`.
              ********************************************************************************
      
      !!
        dist.fetch_build_eggs(dist.setup_requires)
      error in pandas setup command: 'install_requires' must be a string or list of strings containing valid project/version requirement specifiers; Expected end or semicolon (after version specifier)
          pytz >= 2011k
               ~~~~~~~^
      [end of output]

@agoose77
Copy link
Collaborator

@choldgraf 23917da should do it! Not sure why the default is 3.7, but bumped the RTD Python version to 3.10

@choldgraf
Copy link
Member Author

@agoose77 ah shit we just clobbered each other. I didn't realize you had pushed to this branch, sorry about that. I just updated to 3.10 in Sphinx and fixed some bugs that I think were in the pinned dependencies for our docs build (that I think no longer needed pinning). Let's see if that makes things happy.

@choldgraf
Copy link
Member Author

OK readthedocs has some seriously wonky python version action going on. They only seem to support up to 3.8, and don't install it by default, even though one of our dependencies now requires a minimum of 3.8. So I really hope they resolve that one soon!

@choldgraf
Copy link
Member Author

Well @agoose77 I'm back to the install_requires bug. If you wanna work your magic and fix it again that would be awesome, I promise to check and not to clobber your commit this time!

@choldgraf
Copy link
Member Author

choldgraf commented Oct 10, 2023

@agoose77 ahh I see, new config structure. OK I'm stepping away from the computer for a bit so feel free to merge and iterate if you like, otherwise I'll take a look when I get home to see if tests pass

@choldgraf
Copy link
Member Author

Woot tests are happy, let's merge it and see how it impacts all of the other PRs :-) thanks @agoose77 for your help!

@choldgraf choldgraf merged commit 26aa004 into executablebooks:master Oct 10, 2023
11 checks passed
@agoose77
Copy link
Collaborator

I promise to check and not to clobber your commit this time!

I do this all the time!

If you've not seen it, I use git push --force-with-lease, which doesn't force push if the remote has new changes!

@choldgraf
Copy link
Member Author

oh wow I should overwrite git push -f with that haha

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

2 participants