Fix parent of imported category #62
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Since the CIF_MS dictionary is imported in full (head-in-head import), all top level categories from the CIF_MS dictionary become automatically reparented from MS_GROUP to MAGNETIC. Furthermore, the MS_GROUP head category is not imported at all, thus resulting in a missing parent category.
11 tasks
|
I don’t really understand this issue and so have no opinion.
Branton
From: Antanas Vaitkus ***@***.***>
Sent: Friday, January 19, 2024 6:31 AM
To: COMCIFS/magnetic_dic ***@***.***>
Cc: Subscribed ***@***.***>
Subject: [COMCIFS/magnetic_dic] Fix parent of imported category (PR #62)
This PR changes the parent category of the ATOM_SITE_FOURIER_WAVE_VECTOR category from MS_GROUP to MAGNETIC. After the change in the parent category, the ATOM_SITE_FOURIER_WAVE_VECTOR had to be moved to adhere to the dictionary style guide child category ordering rules.
The ATOM_SITE_FOURIER_WAVE_VECTOR category is originally defined in the CIF_MS dictionary and expanded/overwritten in this dictionary. However, since the CIF_MS dictionary is imported in full (head-in-head import), all top level categories from the CIF_MS dictionary become automatically reparented from MS_GROUP to MAGNETIC. Furthermore, the MS_GROUP head category is not imported at all, thus resulting in a missing parent category.
…________________________________
You can view, comment on, or merge this pull request online at:
#62
Commit Summary
* 57eeb1c<57eeb1c> Change parent of ATOM_SITE_FOURIER_WAVE_VECTOR to MAGNETIC.
* f16adeb<f16adeb> Reorder categories in accordance to the style guide.
File Changes
(1 file<https://github.com/COMCIFS/magnetic_dic/pull/62/files>)
* M cif_mag.dic<https://github.com/COMCIFS/magnetic_dic/pull/62/files#diff-b91abc84e0b12625faa7a4050660841de0a83acafc212aab89af01bdae75a698> (384)
Patch Links:
* https://github.com/COMCIFS/magnetic_dic/pull/62.patch
* https://github.com/COMCIFS/magnetic_dic/pull/62.diff
—
Reply to this email directly, view it on GitHub<#62>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACZAIQUJHEK5UHOCNAQXB4LYPJYQVAVCNFSM6AAAAABCB6TAGKVHI2DSMVQWIX3LMV43ASLTON2WKOZSGA4TANJYGEYDGMA>.
You are receiving this because you are subscribed to this thread.Message ID: ***@***.******@***.***>>
|
|
I've merged this in, it is purely a technical issue. Just to explain further, the ATOM_SITE_FOURIER_WAVE_VECTOR category is fully defined in the modulated structures dictionary, and the magnetic dictionary repeats that definition so it can include more examples. Meanwhile, technically speaking, the magnetic structures dictionary views everything found in the modulated structures dictionary as magnetic dictionary definitions. Thus this repeated category, even though it is defined originally in the modulated structures dictionary, should be specified here as belonging to the magnetic dictionary, as that is what all of its modulated structures dictionary definitions end up being. It is a one-line change ultimately. |
|
Hi James,
Could you edit the CIF file I sent to show me exactly how you propose I change it?
Many thanks,
Branton
From: James Hester ***@***.***>
Sent: Monday, January 22, 2024 6:37 PM
To: COMCIFS/magnetic_dic ***@***.***>
Cc: Branton Campbell ***@***.***>; Comment ***@***.***>
Subject: Re: [COMCIFS/magnetic_dic] Fix parent of imported category (PR #62)
Just to explain further, the ATOM_SITE_FOURIER_WAVE_VECTOR category is fully defined in the modulated structures dictionary, and the magnetic dictionary repeats that definition so it can include more examples. Meanwhile, technically speaking, the magnetic structures dictionary views everything found in the modulated structures dictionary as magnetic dictionary definitions. Thus this repeated category, even though it is defined originally in the modulated structures dictionary, should be specified here as belonging to the magnetic dictionary, as that is what all of its modulated structures dictionary definitions end up being. It is a one-line change ultimately.
—
Reply to this email directly, view it on GitHub<#62 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACZAIQXQ5NLUAGCMC5HXJ5LYP4H25AVCNFSM6AAAAABCB6TAGKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBVGEZTINZSGQ>.
You are receiving this because you commented.Message ID: ***@***.******@***.***>>
|
|
I've already made the single, tiny change. On line 55 of https://github.com/COMCIFS/magnetic_dic/blob/main/cif_mag.dic (which is the latest version), line 55 has been changed from If you have any objections to this we can make another PR. |
|
Hi James,
I see. I guess I misunderstood. I thought you wanted a change in all of our magnetic cif files rather than the dictionary itself. Sorry for the confusion. Thank you for your help.
Best wishes,
Branton
Sent via the Samsung Galaxy A53 5G, an AT&T 5G smartphone
…-------- Original message --------
From: James Hester ***@***.***>
Date: 1/23/24 11:15 PM (GMT-07:00)
To: COMCIFS/magnetic_dic ***@***.***>
Cc: Branton Campbell ***@***.***>, Comment ***@***.***>
Subject: Re: [COMCIFS/magnetic_dic] Fix parent of imported category (PR #62)
I've already made the single, tiny change. On line 55 of https://github.com/COMCIFS/magnetic_dic/blob/main/cif_mag.dic (which is the latest version), line 55 has been changed from MS_GROUP to MAGNETIC. All items in the ATOM_SITE_FOURIER_WAVE_VECTOR category have moved to an earlier location in the file to match our ordering style guide, but the actual contents have not changed except for that single change at current line 55.
If you have any objections to this we can make another PR.
—
Reply to this email directly, view it on GitHub<#62 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACZAIQSP2Y2CHQOLUVV3GBLYQCRGRAVCNFSM6AAAAABCB6TAGKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBXGQ2DCMBRHA>.
You are receiving this because you commented.Message ID: ***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR changes the parent category of the
ATOM_SITE_FOURIER_WAVE_VECTORcategory fromMS_GROUPtoMAGNETIC. After the change in the parent category, theATOM_SITE_FOURIER_WAVE_VECTORhad to be moved to adhere to the dictionary style guide child category ordering rules.The
ATOM_SITE_FOURIER_WAVE_VECTORcategory is originally defined in theCIF_MSdictionary and expanded/overwritten in this dictionary. However, since theCIF_MSdictionary is imported in full (head-in-head import), all top level categories from theCIF_MSdictionary become automatically reparented fromMS_GROUPtoMAGNETIC. Furthermore, theMS_GROUPhead category is not imported at all, thus resulting in a missing parent category.