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

Issue 2378 goal: Get CI to pass without ALLOW_UNICODE on doctests #2383

Merged
merged 7 commits into from
Sep 26, 2023

Conversation

ajnelson-nist
Copy link
Contributor

@ajnelson-nist ajnelson-nist commented May 12, 2023

This PR sets the goal state for #2378: Pass CI with a Python2 testing allowance for docstring tests removed.

This PR will be failing CI for a while. My intent is to file separate PRs to main and keep catching this PR up until it eventually passes. I'm aware of at least one patch that will be straightforward, one not so obvious, and am unsure how much further work would be needed.

Fixes #2378

Summary of changes

This PR's sole contribution is expected to be removing ALLOW_UNICODE from pyproject.toml. The rest is expected to be brought in via merging other PRs that lessen the tally of CI failures in this PR.

Checklist

  • Checked that there aren't other open pull requests for
    the same change.
  • Added tests for any changes that have a runtime impact. (N/A)
  • Checked that all tests and type checking passes.
  • For changes that have a potential impact on users of this project:
    • Updated relevant documentation to avoid inaccuracies. (N/A)
    • Considered adding additional documentation. (N/A)
    • Considered adding an example in ./examples for new features. (N/A)
    • Considered updating our changelog (CHANGELOG.md).
  • Considered granting push permissions to the PR branch,
    so maintainers can fix minor issues and keep your PR up to date.

This patch, without rebasing, will fail CI today.

References:
* RDFLib#2378

Signed-off-by: Alex Nelson <alexander.nelson@nist.gov>
References:
* RDFLib#2378

Signed-off-by: Alex Nelson <alexander.nelson@nist.gov>
@aucampia
Copy link
Member

Thanks for the PR @ajnelson-nist, would be good to eliminate ALLOW_UNICODE for doctests.

While I do prefer smaller PRs, I would not mind if you did everything for this in one PR for this, as it should not be too difficult to review. But also happy if they go into seperate PRs.

@aucampia aucampia added the needs more work The PR needs more work before being merged or further reviewed. label May 19, 2023
@ajnelson-nist
Copy link
Contributor Author

@aucampia sounds fine to me. Should #2384 be merged into main in the meanwhile, or should I just merge it into this branch and close that PR?

@ajnelson-nist
Copy link
Contributor Author

Oh wait, I forgot part of my motivation for carving out other PRs: I have smacked into at leas one issue that will require slightly harder work (handling an inequality operator in Literal vs. URIRef), so I wanted to do the less-thinking-required efforts in other PRs, and focused fixes in other PRs.

@aucampia
Copy link
Member

@aucampia sounds fine to me. Should #2384 be merged into main in the meanwhile, or should I just merge it into this branch and close that PR?

I will check it shortly, I think so though.

@aucampia
Copy link
Member

@aucampia sounds fine to me. Should #2384 be merged into main in the meanwhile, or should I just merge it into this branch and close that PR?

I will check it shortly, I think so though.

Approved and merged. Apologies for taking so long to get to it, for small PRs like #2384 you should consider asking on Gitter/Matrix. I try to process PRs in order, and sometimes I don't even look at new PRs before older ones are processed, if I knew it was so small I would have processed it sooner.

@aucampia aucampia added documentation Improvements or additions to documentation fix Fixes an issue on hold Progress on this issue is blocked by something. and removed needs more work The PR needs more work before being merged or further reviewed. labels May 19, 2023
@aucampia
Copy link
Member

PRs to V6 is closed until further notice. See this for more details:

@aucampia
Copy link
Member

PRs to V6 is closed until further notice. See this for more details:

We will be open for PRs again once this is resolved:

@aucampia aucampia removed the on hold Progress on this issue is blocked by something. label Jun 2, 2023
aucampia added a commit to aucampia/rdflib that referenced this pull request Sep 25, 2023
Unicode literals (`u'ẋẏż'`) are redundant in Python 3 as all strings
support Unicode. This change eliminates the use of Unicode literals from
docstrings so that the `ALLOW_UNICODE` option flag is no longer needed
for doctests.

See also:
- RDFLib#2383
- RDFLib#2378
aucampia added a commit to aucampia/rdflib that referenced this pull request Sep 25, 2023
Unicode literals (`u'ẋẏż'`) are redundant in Python 3 as all strings
support Unicode. This change eliminates the use of Unicode literals from
docstrings so that the `ALLOW_UNICODE` option flag is no longer needed
for doctests.

See also:
- RDFLib#2383
- RDFLib#2378
aucampia added a commit that referenced this pull request Sep 25, 2023
Unicode literals (`u'ẋẏż'`) are redundant in Python 3 as all strings
support Unicode. This change eliminates the use of Unicode literals from
docstrings so that the `ALLOW_UNICODE` option flag is no longer needed
for doctests.

See also:
- #2383
- #2378
@coveralls
Copy link

Coverage Status

coverage: 90.941%. remained the same when pulling f19ee60 on ajnelson-nist:issue-2378-goal into cc7350f on RDFLib:main.

@aucampia aucampia merged commit b0392f0 into RDFLib:main Sep 26, 2023
22 checks passed
@ajnelson-nist ajnelson-nist deleted the issue-2378-goal branch October 3, 2023 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation fix Fixes an issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is the pytest+doctest configuration ALLOW_UNICODE worth keeping?
3 participants