-
Notifications
You must be signed in to change notification settings - Fork 89
Add ability to select path segments using Python re regexprs; follow on to PR#146 #177
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
Conversation
Recovered 1) tests/test_path_ext_py from PR submission 2) some github workflows used to validate PR submission
Follow-up PR: "Add ability to select path segments using Python re regexprs" dpath-maintainers#146 on June 1, 2021 Includes: - integration of extension on dpath-python - improvement of tests (under nose2, with scripts test/noseRunner to control code options) - improvement of documentation in README.rst Baseline for integration: commit ea64635 (tag: v2.1.1, origin/master, origin/HEAD, master, AL-master4merge) Merge: 702b5dd 38007df Author: moomoohk <[email protected]> Date: Wed Nov 30 17:16:05 2022 +0200 Note: at this time, no test wrt. Github.Actions has been made (yet)
Resolved conflicts: dpath/segments.py dpath/version.py Testing to be redone!
- tests improved and running locally - documentation updated Follow-up PR: "Add ability to select path segments using Python re regexprs" dpath-maintainers#146 on June 1, 2021 Baseline for merge: commit 45b3488 (tag: v2.1.2, origin/master, origin/HEAD, master) Author: moomoohk <[email protected]> Date: Mon Dec 5 09:52:05 2022 +0200
…ng action manually
…on enhancements 1) suppress dependency on mock now integrated as unittest.mock 2) avoids warnings concerning invalid escape sequences: either use raw strings or use double backslash: testing both 3) detailed the issue of such escape sequences in re.regexp in README.rst, giving example of both techniques No change in .yml: rerunning should resolve identified issues
…path for pypy - Improved Github actions (clean up, test under pypy3) - formatting in accordance with Flake8, deal with errors diagnosed by Flake but accepted by python3 - moved test tools to dir test-utils - tox.ini, flake.ini: avoid spurious diagnostics
…ing StringMatcher capability: 1) supports extension by re.regex (re.Pattern.match) 2) supports duck typed Duck_StringMatcher derived classes which permit user defined pattern matcher 3) included tests and documenation 4) dealt with Flake8 diagnostics Also found a random rare error in test_segments.test_view, left Pdb obtained information in the source file (see bottom of tests/test_segments.py). This corresponds with an unrealistic situation random generated by hypothesis, not easily reproducible for lack of random seed.
…ngMatcher extension
Hi @moomoohk
|
Hi Alain, If the core functionality (adding regexes to paths) works well enough then I'd like to restructure this PR. I hope that's ok with you! Perhaps the other changes can be considered separately. Thanks. |
Hi,
a) I will be pushing a version that cleans up the functionality you
probably are not interested in (using a class or a function to define
matching).
b) doing this, I have cleared the issues with running on (slightly) older
versions of the Python or Pypy (which was more difficult/surprising).
c) having a lot of tests helped, especially because the type
identification for instances is subtle.
So you should probably start with this forthcoming version, to clean up
and see what you want to keep.
For me, it is OK that you keep only the functionality you are interested
in.
( Personally, I got interested in the example with Levenshtein
approximate matching, when it seemed clear that this would be such a small
change)
Looking forwards to reading you
Le mar. 10 janv. 2023 à 21:51, moomoohk ***@***.***> a écrit :
… Hi Alain,
I've cleared enough from the backlog and now I'd like to focus on your PR.
After reviewing the changes you've made it's clear a lot of work has been
put in and that's much appreciated.
That said, I feel that a lot of the changes included aren't within the
scope of the feature addition you're proposing.
If the core functionality (adding regexes to paths) works well enough then
I'd like to restructure this PR. I hope that's ok with you!
Perhaps the other changes can be considered separately.
Thanks.
—
Reply to this email directly, view it on GitHub
<#177 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACWFOIQCUDDT25O7WUHZQI3WRXDUHANCNFSM6AAAAAAS5CSIO4>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Hi @moomoohk,
After thinking about it, I think it will be simpler for you if I committed:
a) a simplified version with only re regexp in the branch linked to the PR.
This will keep only that functionality plus workarounds for working Pypy if
required.
b) another full featured version in a new branch, which includes what I
have now and can be readied for a PR after some testing in Github's
containers
This plan should be simpler for you, since I know what I put in!
Have a good day !😀
Le mar. 10 janv. 2023 à 22:35, Alain Lichnewsky ***@***.***>
a écrit :
… Hi,
a) I will be pushing a version that cleans up the functionality you
probably are not interested in (using a class or a function to define
matching).
b) doing this, I have cleared the issues with running on (slightly) older
versions of the Python or Pypy (which was more difficult/surprising).
c) having a lot of tests helped, especially because the type
identification for instances is subtle.
So you should probably start with this forthcoming version, to clean up
and see what you want to keep.
For me, it is OK that you keep only the functionality you are interested
in.
( Personally, I got interested in the example with Levenshtein
approximate matching, when it seemed clear that this would be such a small
change)
Looking forwards to reading you
Le mar. 10 janv. 2023 à 21:51, moomoohk ***@***.***> a
écrit :
> Hi Alain,
> I've cleared enough from the backlog and now I'd like to focus on your PR.
> After reviewing the changes you've made it's clear a lot of work has been
> put in and that's much appreciated.
> That said, I feel that a lot of the changes included aren't within the
> scope of the feature addition you're proposing.
>
> If the core functionality (adding regexes to paths) works well enough
> then I'd like to restructure this PR. I hope that's ok with you!
>
> Perhaps the other changes can be considered separately.
>
> Thanks.
>
> —
> Reply to this email directly, view it on GitHub
> <#177 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ACWFOIQCUDDT25O7WUHZQI3WRXDUHANCNFSM6AAAAAAS5CSIO4>
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>
|
Deals with - issues testing with pypy (an possibly some older Pythons) - flake8 diagnostics Other developments to go to separate PR
Hi @moomoohk a) this is fine, do you expect me to do anything? Something like the following should work (not tried):
Have a good day :-) |
Hi Alain
Hi Alain, Thanks and regards |
# https://peps.python.org/pep-0604/#isinstance-and-issubclass | ||
# https://bugs.python.org/issue44529 | ||
|
||
try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this check needed for?
@@ -6,6 +6,12 @@ | |||
from dpath.exceptions import InvalidGlob, InvalidKeyName, PathNotFound | |||
from dpath.types import PathSegment, Creator, Hints, Glob, Path, SymmetricInt | |||
|
|||
import re | |||
try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dpath's minimum supported version is Python 3.7 making this check redundant
Hi @moomoohk, For now: Regards |
Hi @moomoohk You were right to spot not needed stuff, some that can be assumed for Python >= 3.7, others that were left from my exercise with duck-typed generalization. Regards |
May we close this PR for the time being? |
Hi, this is a follow-on of my previous PR (#146) on same subject; apparently it was simpler for me to use a different branch.
This has run Action https://github.com/AlainLich/dpath-python/actions/runs/3684590771 in my branch
AL-master4merge
. I have added tests exercising this with most functions, and documented inREADME.rst
All details are in the list of commits, including documentation, tests,...
I hope this suits your needs, let me know if there are still issues for merging in the main baseline.