Skip to content

Coping with different versions of AMOF standard#26

Merged
joshua-hampton merged 4 commits into
mainfrom
specs-fmt-checks-etc-hampton
Jun 5, 2023
Merged

Coping with different versions of AMOF standard#26
joshua-hampton merged 4 commits into
mainfrom
specs-fmt-checks-etc-hampton

Conversation

@joshua-hampton

@joshua-hampton joshua-hampton commented May 19, 2023

Copy link
Copy Markdown
Collaborator

Moved NCAS-AMOF specs and vocabs into folders based on standard release version.
When checking an NCAS-AMOF file, checksit:

  • looks for NCAS-GENERAL, NCAS-AMOF or NCAS-AMF in conventions
  • identifies which version of the standard is being used
  • uses that version number to find correct specs folder

Changes also to make_specs.py:

  • code now encased within a function that takes version_number as a parameter
  • looks for vocabs in f"checksit/vocabs/AMF_CVs/{version_number}" and writes specs to f"specs/groups/ncas-amof-{version_number}"

Could include call to make_specs, and some code that pulls the vocab CVs from github, from checksit/check - if standard version identified isn't present in checksit, go looking to see if we can make the specs?

@agstephens agstephens left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@joshua-hampton: this looks like a straightforward and sensible approach.

On the issue of whether you have a mechanism to go and look for versions that are not found:

  • if that is easy to do then you could try it
  • but, installations might exist where the user cannot write to the installed version of checksit
  • so, a fall-back could be "please talk to your Admin about getting version {version} installed (if it exists)"

@agstephens

Copy link
Copy Markdown
Member

@joshua-hampton , when you have finalised this change and merged it, let's increment the version of checksit as a tagged release.

@joshua-hampton

joshua-hampton commented May 24, 2023

Copy link
Copy Markdown
Collaborator Author

I think I have implemented this. In check.py, upon discovering the file in question is an NCAS-AMOF file, checksit will:

  • determine what version of the standard is being used in the file
  • look to see if specs already exist in specs/groups/ncas-amof-X.X.X
  • if yes, continue as before
  • if no:
    • attempt to make folders in vocabs and specs for the new version
    • downloads all the json files from https://github.com/ncasuk/AMF_CVs/tree/vX.X.X/AMF_CVs, writing to the vocabs folder
    • run make_specs.make_amof_specs(X.X.X) to convert vocabs into spec files

A couple of exceptions explicitly caught and raised:

  • urllib.error.HTTPError - unable to download vocabs. This could either be because the user has no internet connection, or they are trying to download a version that does not exist
  • PermissionError - unable to make folders or write files

To help this work, make_specs.py has been moved from top level in the repo to within the checksit/ folder.

In doing this, I also discovered that some of the vocab and spec files I had were different than what would be made for v2.0.0 in this method. I had files that I had downloaded myself from the spreadsheets when testing that code, presumably the sheets had been changed since the release version in AMF_CVs. I have updated the ones in checksit to match the v2.0.0 release in AMF_CVs.

@joshua-hampton joshua-hampton requested a review from agstephens May 24, 2023 08:18

@agstephens agstephens left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@joshua-hampton: this looks great. Please merge - and I'll install on JASMIN.

@joshua-hampton joshua-hampton merged commit 7136628 into main Jun 5, 2023
@joshua-hampton joshua-hampton deleted the specs-fmt-checks-etc-hampton branch March 11, 2026 13:45
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.

2 participants