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 ENUMERATION_SOURCE #406

Merged
merged 14 commits into from
Jun 7, 2023
Merged

Conversation

rowlesmr
Copy link
Collaborator

@rowlesmr rowlesmr commented Jun 2, 2023

Will close #402

Add ability to record the source of the values in a default enumeration (ie the ones in templ_enum.cif) to be recorded.

ddl.dic Outdated Show resolved Hide resolved
Co-authored-by: Antanas Vaitkus <antanas.vaitkus90@gmail.com>
Copy link
Collaborator

@vaitkus vaitkus left a comment

Choose a reason for hiding this comment

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

Thank you for the work. I suggested a few rewrites which seem to bring the definitions closer to other definitions in the dictionary. Everything else seems ok, but I will take a more proper look in the evening.

ddl.dic Outdated Show resolved Hide resolved
ddl.dic Show resolved Hide resolved
ddl.dic Outdated Show resolved Hide resolved
ddl.dic Outdated Show resolved Hide resolved
ddl.dic Outdated Show resolved Hide resolved
ddl.dic Outdated Show resolved Hide resolved
rowlesmr and others added 7 commits June 3, 2023 12:36
Co-authored-by: Antanas Vaitkus <antanas.vaitkus90@gmail.com>
Co-authored-by: Antanas Vaitkus <antanas.vaitkus90@gmail.com>
Co-authored-by: Antanas Vaitkus <antanas.vaitkus90@gmail.com>
Co-authored-by: Antanas Vaitkus <antanas.vaitkus90@gmail.com>
so you don't need to have an id if you're just giving one reference
Co-authored-by: Antanas Vaitkus <antanas.vaitkus90@gmail.com>
ddl.dic Outdated Show resolved Hide resolved
ddl.dic Outdated Show resolved Hide resolved
@vaitkus vaitkus mentioned this pull request Jun 3, 2023
a specific enumeration value.
;
_name.category_id enumeration_default
_name.object_id source_id
Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't we now using ENUMERATION_DEFAULTS? (S on the end)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's in #404, and not merged yet.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, #399

@jamesrhester
Copy link
Contributor

Looks fine, just needs to be rewritten in terms of enumeration_defaults

@rowlesmr
Copy link
Collaborator Author

rowlesmr commented Jun 5, 2023

Looks fine, just needs to be rewritten in terms of enumeration_defaults

Roger that. Just need enumeration_defaults merged in first.

@vaitkus
Copy link
Collaborator

vaitkus commented Jun 5, 2023

@jamesrhester @rowlesmr Maybe we could merge this PR as is and then simply update it once the ENUMERATION_DEFAULTS changes are resolved/merged?

After all, the ENUMERATION_SOURCE can be used to describe the sources of both the ENUMERATION_DEFAULT and ENUMERATION_DEFAULTS.

@jamesrhester jamesrhester merged commit c110173 into COMCIFS:master Jun 7, 2023
3 checks passed
@jamesrhester
Copy link
Contributor

Agreed and merged.

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.

Add ability to formally record enumeration source?
3 participants