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.

Sign up for GitHub

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

Jump to bottom

Test if OME fields are in JSON-LD OME context #29

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Test if OME fields are in JSON-LD OME context #29

wants to merge 5 commits into from

Conversation

stefanches7
Copy link
Contributor

No description provided.

joshmoore
joshmoore reviewed Dec 10, 2024
View reviewed changes
Copy link
Member

@joshmoore joshmoore left a comment

Choose a reason for hiding this comment

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

Few intermediate comments while you hammer away.

src/omero_rdf/__init__.py
OME = "http://www.openmicroscopy.org/rdf/2016-06/ome_core/"
OMERO = "http://www.openmicroscopy.org/TBD/omero/"
OME = "ome:"
OMERO = "ome:"
Copy link
Member

Choose a reason for hiding this comment

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

Think I would differentiate here. i.e., either we need no OMERO = or leave it as omero:. I'm thinking keep OMERO and will have to split https://gist.githubusercontent.com/stefanches7/5b3402331d901bb3c3384bac047c4ac2/raw/cd45da585bfa630a56ef55670d2b5da2be50ff76/context.ld.json into two files (before we're done)

src/omero_rdf/__init__.py Outdated
if '@context' in data:
return data['@context']
else:
print(f"No @context found in {url}")
Copy link
Member

Choose a reason for hiding this comment

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

Need to use logging.warning to prevent losing stdout

Copy link
Contributor Author

Choose a reason for hiding this comment

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

implementing it right now

src/omero_rdf/__init__.py Outdated
"""
try:
# Make HTTP request
response = requests.get(url, headers={'Accept': 'application/ld+json'})
Copy link
Member

Choose a reason for hiding this comment

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

We really want to have this document built into the library so it need not be downloaded remotely but that can be phase 2.

cf. https://github.com/hyperledger-archives/aries-framework-go/blob/main/docs/jsonld_context.md

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So we'd keep a local version of the context, or how? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

In XML-land, you create a catalog.xml (https://github.com/ome/ome-model/blob/908c6ef501f18b329d46a65fb7bd563f0d5580a6/specification/src/main/resources/released-schema/catalog.xml#L5) then you put it into your Java jar (for example) and you never have to hit the network.

@joshmoore
Copy link
Member

Updated the branch to origin/main, resolved conflicts and flake8 warnings.

At this stage I'm seeing:

WARNING:root:Did not find in OMERO context: @context. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: @vocab. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: omero. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: AcquisitionDate. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: Description. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: omero:series. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: @context. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: @vocab. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: omero. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: PhysicalSizeX. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: Symbol. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: Unit. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: Value. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: PhysicalSizeY. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: Symbol. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: Unit. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: Value. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: SignificantBits. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: SizeC. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: SizeT. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: SizeX. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: SizeY. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: SizeZ. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: Type. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: omero:sha1. Add it to the spreadsheet of new fields

@joshmoore
Copy link
Member

Merging in #30 and #32 I see:

WARNING:root:Did not find in OMERO context: AcquisitionDate. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: Description. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: omero:series. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: PhysicalSizeX. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: Symbol. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: Unit. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: Value. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: PhysicalSizeY. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: Symbol. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: Unit. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: Value. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: SignificantBits. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: SizeC. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: SizeT. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: SizeX. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: SizeY. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: SizeZ. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: Type. Add it to the spreadsheet of new fields
WARNING:root:Did not find in OMERO context: omero:sha1. Add it to the spreadsheet of new fields

@joshmoore joshmoore marked this pull request as draft December 12, 2024 09:23
@joshmoore
Copy link
Member

A heads up that I think this PR will ultimately be more for debugging. I'd really like to find a strategy so we're not requests-ing the JSON-LD on each call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joshmoore joshmoore joshmoore left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@stefanches7 @joshmoore