search

COMCIFS / magnetic_dic

Development of the magnetic CIF dictionary
0 stars 4 forks source link

Fix parent of imported category #62

Closed vaitkus closed 7 months ago

vaitkus commented 7 months ago

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.

brantonc commented 7 months ago

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:

https://github.com/COMCIFS/magnetic_dic/pull/62

Commit Summary

File Changes

(1 filehttps://github.com/COMCIFS/magnetic_dic/pull/62/files)

Patch Links:

— Reply to this email directly, view it on GitHubhttps://github.com/COMCIFS/magnetic_dic/pull/62, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ACZAIQUJHEK5UHOCNAQXB4LYPJYQVAVCNFSM6AAAAABCB6TAGKVHI2DSMVQWIX3LMV43ASLTON2WKOZSGA4TANJYGEYDGMA. You are receiving this because you are subscribed to this thread.Message ID: @.**@.>>

jamesrhester commented 7 months ago

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.

brantonc commented 7 months ago

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 GitHubhttps://github.com/COMCIFS/magnetic_dic/pull/62#issuecomment-1905134724, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ACZAIQXQ5NLUAGCMC5HXJ5LYP4H25AVCNFSM6AAAAABCB6TAGKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBVGEZTINZSGQ. You are receiving this because you commented.Message ID: @.**@.>>

jamesrhester commented 7 months ago

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.

brantonc commented 7 months ago

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 GitHubhttps://github.com/COMCIFS/magnetic_dic/pull/62#issuecomment-1907441018, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ACZAIQSP2Y2CHQOLUVV3GBLYQCRGRAVCNFSM6AAAAABCB6TAGKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBXGQ2DCMBRHA. You are receiving this because you commented.Message ID: @.***>