filter.py - Allow tags to be specified using a string containing a 0x-prefix hex number

[howff]

Description

Allow tags to be specified using a string containing a 0x-prefix hex number

Related issues: #253

Converts a field name which is a string in the form '0xGGGGEEEE' (GroupElement) into a number so that it can be found in the dataset dict. This allows recipe rules to specify tags numerically. One use for this is to filter on private tags.

Checklist

• I have commented my code, particularly in hard-to-understand areas
• My changes generate no new warnings
• My code follows the style guidelines of this project

Open questions

Questions that require more discussion or to be addressed in future development:

Need to consider the implications of #205 (Finding relocated private elements using "PrivateCreator")

[howff]

I've also updated the documentation, CHANGELOG, and added a new test (with associated recipe file).

[vsoch]

This is a great start! We will want to bump the version. Also ping @wetzelj to discuss the tests he thinks would be good to add.

[vsoch]

This overall looks great - the formatting is failing because you likely don't have black==23.3.0 locally. My one question is if we want a test or two more? From @wetzelj:

Just glancing at the code, I'm wondering if converting the field to an int will be enough. Does this correctly retrieve the private tag field in all situations? This change would be a great opportunity to add some private tag unit tests to test_filter_detect.py. These tests were initially added focusing on the interplay between two conditions within a rule (#142), but it would be beneficial to add more atomic tests for private/public tags and each of the filter types.

Specifically this part:

it would be beneficial to add more atomic tests for private/public tags and each of the filter types.

@wetzelj do you think more tests are warranted here, or was this scoped to another follow up PR?

[howff]

I've added a test specifically for the new feature, so I guess if you want different tests then maybe they could be in a different issue/pr?

[wetzelj]

I haven't had a chance to look at this today and don't expect to be able to circle back to it until Friday, but I would also contend that it's important to test that this new feature didn't unintentionally break existing expected functionality.

[wetzelj]

This should be okay. Given that the other tests check the string method, they're already validating existing functionality. Thanks for your work @howff!

[vsoch]

Thank you @howff !

[wetzelj]

@vsoch, @howff - Can this be merged and completed?

[vsoch]

This was my bad! I was probably waiting for tests to finish and my browser crashed and I forgot to check again. Thanks for the reminder @wetzelj and apologies @howff!

[vsoch]

Thank you! https://pypi.org/project/deid/0.3.22/