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

Maintenance update #178

Closed
wants to merge 8 commits into from
Closed

Maintenance update #178

wants to merge 8 commits into from

Conversation

Drvanon
Copy link
Contributor

@Drvanon Drvanon commented Mar 26, 2023

This branch bumps some versions:

  • Python ^3.6 -> ^3.7
  • pytest ^2.4 -> ^7.0
  • pytest-cov ^2.8.1 -> ^4.0
  • covervage <5.0 -> ^5.0
  • black 19.10b0 -> ^23.1

Then I ran poetry update in order to ensure that the poetry.lock file was up to date.

Using this configuration I was able to run poetry install without running in to issue #176, which makes me expect that that that should be resolved, but maybe I do not understand that well enough. It is either that, or the fact that they merged an update last week.

I could run poetry run pytest --cov=functional without issues, getting the same coverage as is currently posted on the repository. I am having issues with Travis, which is giving me trouble running the build.

The new version of black suggests small changes in the tests files, namely the removal of the u-strings and minor formatting stuff for powers. As far as I can tell this is fine. Pylint also complains about the u-strings and then some formatting which was being done in python 3.6 style (using .format as opposed to f strings). Pylint also suggests to specifying the file format when using open in the tests. I can inlcude these suggestions in the PR if you would like me to, though they are all warning level messages.

I would like to mention that there is still a travis.sh file in the project root, however as far as I can tell that is from an outdated version of Travis. Would it make sense to remove it in this in this PR?

@Drvanon
Copy link
Contributor Author

Drvanon commented Mar 26, 2023

After a shower and a good walk I decided to go ahead and do it anyways. I tried to make my commits atomic, so you can also revert individual bits if you like.

Note that this was run with --ignore-missing-imports.
@Drvanon Drvanon mentioned this pull request Mar 26, 2023
6 tasks
@EntilZha
Copy link
Owner

EntilZha commented Apr 3, 2023

It looks like github no longer has testing for python3.6, could you remove that from the workflows to see if everything passes? Given its a maintenance update, you could also remove notation of 3.6 support from readme since I believe its past EOL?

Thanks for the update, I'll release a new version on pypi once merged.

@Drvanon
Copy link
Contributor Author

Drvanon commented Apr 16, 2023

Since the original README stated that it tested with PyPy as well, I included pypy3.9 in the strategy matrix. I think it makes sense to test for all supported python versions, I added those. I also removed references to Python 3.6 where they are no longer accurate.

@Drvanon
Copy link
Contributor Author

Drvanon commented Apr 16, 2023

About publishing to PyPi: I am not sure what strategy you would like to adhere to concerning the typing update. I see the following possibilities:

  1. Create a version 1.4.4 with only the maintenance updates. Then release the typing stuff with 1.4.5.
  2. Wait until I finish the typing update and package both updates as 1.4.

@artemisart
Copy link
Contributor

artemisart commented May 14, 2023

@Drvanon Hello, I believe the CI fails as the setup-python action is too old and cannot install pypy39, can you try using actions/setup-python@v4 ? actions/checkout@v3 would be nice too.
Also files are utf8, not us-ascii.

@EntilZha
Copy link
Owner

Re pypy, I'm fine with essentially having two tiers of supported versions: (1) tested with CI, etc so mainly mainline python and (2) not CI, but best effort compatibility. That said, if the fix is to just use a newer version of actions, that works too! Either way, happy to merge once CI passes.

@artemisart
Copy link
Contributor

@Drvanon or @EntilZha can you rebase this PR to have the updated actions? (I don't have access to do it on this PR/branch)

@artemisart artemisart mentioned this pull request Jun 21, 2023
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