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

Add UUID generator incorporating dictionary entry keys #164

Conversation

sldouglas-nist
Copy link

No description provided.

@sldouglas-nist sldouglas-nist requested a review from a team as a code owner August 21, 2024 20:06
@ajnelson-nist ajnelson-nist marked this pull request as draft August 21, 2024 20:15
Reviewed-by: Alex Nelson <alexander.nelson@nist.gov>
Signed-off-by: Sheldon Douglas <sheldon.douglas@nist.gov>
@sldouglas-nist sldouglas-nist force-pushed the add_dictionary_entry_inherent_iri_generator branch from f8d9851 to 972bf74 Compare August 21, 2024 20:19
@ajnelson-nist ajnelson-nist marked this pull request as ready for review August 21, 2024 20:20
@ajnelson-nist
Copy link
Member

CI should finish before this merges.

@@ -96,6 +96,15 @@
)


def dictionary_entry_inherence_uuid(
uco_object_uuid_namespace: uuid.UUID, key_name: str, *args: Any, **kwargs: Any
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why accept *args and **kwargs if they're silently ignored?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point. These were my suggestion, so I'll field the question.

This addresses a syntax in Python's function definitions that frequently looks ambiguous to me, where a kind-of-positional spelling can be used at call time.

I've found it helpful to have at least *args always, as a reminder that there's a point in the argument list where the arguments are no longer positional. If it comes at the end, great! All arguments are positional. **kwargs tends to come along as a matter of habit - if there's *args, I tend to want **kwargs as well.

I also include *args and **kwargs in class methods when I know subclasses are going to override the implementations, or use super().

So, by two habits, I use *args and **kwargs in function definitions, and I suggested those to @sldouglas-nist .

@ajnelson-nist
Copy link
Member

It looks like a part of the build system post-tests needs fixing. FWIW, it seems out of scope of the changes in this PR, so I think it would be fine to merge this PR, but I'm also fine waiting for CI to pass with another PR merged into develop first.

@kchason kchason merged commit bf4da9a into casework:develop Aug 21, 2024
1 of 2 checks passed
@ajnelson-nist ajnelson-nist added this to the 0.17.0 milestone Aug 26, 2024
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