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

Added _database_code.BIncStrDB. #486

Merged
merged 3 commits into from
Mar 28, 2024
Merged

Conversation

nautolycus
Copy link
Collaborator

No description provided.

@nautolycus
Copy link
Collaborator Author

This follows a request by Gotzon Madariaga: "it would improve the interoperability among other databases and also enhance the visibility of the database, calling the attention of publishers to contact us". Note that the Database formally uses the abbreviation B-IncStrDB ( which I have given in the description), but I have used an object_id without a hyphen (to minimise confusion between hyphens and underscores). I'm open to being persuaded that having two forms of the abbreviation in the dictionary might cause more confusion. I've also added in _alias.definition_id values in case @vaitkus thinks it useful to add these to the legacy core dictionary (this could be beneficial for people currently working in the CIF1/DDL1 world).

cif_core.dic Show resolved Hide resolved
cif_core.dic Outdated Show resolved Hide resolved
cif_core.dic Outdated Show resolved Hide resolved
@vaitkus
Copy link
Collaborator

vaitkus commented Mar 27, 2024

@nautolycus I tried to address the main points that you raised in separate review comments to keep things slightly more organised.

One additional thing that I would recommend is to add B-IncStrDB to the list of databases in the save_database_list save frame of the templ_enum.cif file. This way it will become available for cross-linking via the _database_related.database_id data item.

@nautolycus
Copy link
Collaborator Author

@vaitkus Thanks for your careful review (and advice). I'm consulting with Gotzon as to whether he is happy to have the hyphen-less abbreviation in the tags (and _enumeration_set.state for _database_related.database_id), and will revise accordingly.

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.

Looks great, thanks!

Happy to merge as is, but I do have one more question. I was unable to find online references to the new database name (Bilbao Modulated Structures Database), however, I assume that it came directly from the database maintainers?

@nautolycus
Copy link
Collaborator Author

Yes, Gotzon Madariaga is one of the maintainers and an author of the commentary chapter in Volume G. The current draft of that has a note that they are moving away from the "Incommensurate" term in the title of the database, but retaining the B-IncStrDB abbreviation for historical reasons. I have just prompted him to consider changing the title on the database web pages :)

@vaitkus
Copy link
Collaborator

vaitkus commented Mar 28, 2024

Great, no reason not to merge then. Thanks!

@vaitkus vaitkus merged commit dcb6dc4 into COMCIFS:master Mar 28, 2024
3 checks passed
@nautolycus nautolycus deleted the dbcode_update branch March 28, 2024 16:20
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.

None yet

2 participants