lcompilers / lpython

Python compiler
https://lpython.org/
Other
1.37k stars 157 forks source link

Added APIs `isalnum()` and `isnumeric()` in string object #2572

Closed Kishan-Ved closed 4 months ago

Kishan-Ved commented 4 months ago

Towards: https://github.com/lcompilers/lpython/issues/2356

APIs added:

These have been tested extensively, for both compile time and run time tests.

Kishan-Ved commented 4 months ago

I'll resolve all the conflicts here (if any) once the review is done.

Kishan-Ved commented 4 months ago

I shared a query above. The rest seems good to me. Conflicts need to be resolved. I will review again after conflicts are resolved.

Please let me know a fast way to resolve such conflicts. These have appeared as I had changed references in this PR: https://github.com/lcompilers/lpython/pull/2574, which has now been merged. I had made both these PRs by branching from the main branch that existed before I made that PR.

Shaikh-Ubaid commented 4 months ago

Please let me know a fast way to resolve such conflicts.

I always commit updation of reference tests in a separate commit (with the fixed message "TEST: Update reference tests"). That way if there are conflicts between the reference test updates of my branch and the main branch, I can just drop my commit "TEST: Update reference tests" and then rebase over the main branch. After rebase, I simply rebuild the project, update the reference tests and add a new commit with the updated reference tests (with the same commit message "TEST: Update reference tests").

Shaikh-Ubaid commented 4 months ago

Please let me know a fast way to resolve such conflicts.

For now, maybe try this (make a separate branch before trying so that you can always recover your changes).

git rebase --strategy-option ours main
git clean -dfx
./build.sh
./run_tests.sh -u
git add tests
git commit -m "TEST: Update reference tests"
Shaikh-Ubaid commented 4 months ago

For now, maybe try this (make a separate branch before trying so that you can always recover your changes).

If this does not work, a simple option could be to create a new branch from main and manually add the changes in this PR to it. Finally update and commit the reference tests in a separate commit.

There can also be another option in which you rebase and edit your commit Implemented isnumeric() to only include changes related to source code and exclude changes related to reference tests. Next you drop your commits related to changes in reference tests (using git rebase again) and finally rebase over main. Now you can update tests and commit them without any conflicts.

Thirumalai-Shaktivel commented 4 months ago

Please mark this PR ready for review once it is ready.

Kishan-Ved commented 4 months ago

As there were too many conflicts, I have created a new PR: https://github.com/lcompilers/lpython/pull/2582

Kishan-Ved commented 4 months ago

Finished and merged: https://github.com/lcompilers/lpython/pull/2582