-
Notifications
You must be signed in to change notification settings - Fork 3
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
Create python package #3
base: main
Are you sure you want to change the base?
Conversation
hgoerzig
commented
Aug 18, 2021
•
edited
Loading
edited
- renamed folder skript to code
- separated jupyther code from python code
- created a python package (nxsOnto) added requirements.txt and setup.py
- removed some comments in the code and renamed file
Please edit this to provide a proper title and description |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tree does not need to be so deep.
Remove the empty README.md.
code/nxsOnto/setup.py
Outdated
packages=['nxsOnto'], | ||
license='Apache 2', | ||
install_requires=[ | ||
"owlready2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This duplicates requirements.txt so should read that file
code/nxsOnto/setup.py
Outdated
version='1.1', | ||
description='Generates an ontology from nxdl files', | ||
author='Steve Collins', | ||
url="https://github.com/nexusformat/NeXusOntology/script", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does not exist
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The README does not explicitly say how I run this and what the outcome is. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've add more comments from #2
Also please delete the notebook.
code/nxsOnto/nxsOnto/generator.py
Outdated
@@ -106,25 +106,31 @@ | |||
The version string is the NeXus version followed by the ontology version. | |||
|
|||
''' | |||
|
|||
''' | |||
# To avoid re-parsing NeXus files after initial run, execute this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of this is commented out code! Not sure why you want it too as you run this script once.
code/nxsOnto/nxsOnto/generator.py
Outdated
g = Github(token) | ||
repo = g.get_repo(nexus_repo) | ||
|
||
with urllib.request.urlopen(repo.tags_url) as url: | ||
tags = json.loads(url.read().decode()) | ||
tagsDict = tags[0] # get version tags from master branch | ||
|
||
|
||
base_class_url = [] | ||
for file in repo.get_contents("base_classes"): | ||
if str(file).split('.')[-2] == 'nxdl': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if file name does not have two '.'?
code/nxsOnto/nxsOnto/generator.py
Outdated
|
||
def addFieldToDict(classDict, field, defn_name): # make a function to be reused later | ||
#defn_name is used to add application definition to field dict if the field is defined in an app deff. | ||
field_name = field.getAttribute('name') | ||
|
||
|
||
deprecationAttribute = field.getAttribute('deprecated') | ||
if not deprecationAttribute == '': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a Pythonic way to test for an empty string
code/nxsOnto/nxsOnto/generator.py
Outdated
deprecationAttribute = field.getAttribute('deprecated') | ||
if not deprecationAttribute == '': | ||
print("=== Deprecation warning %s in %s: %s" % (field_name, className, deprecationAttribute)) | ||
print("=== Deprecation warning %s in %s: %s" % (field_name, className, deprecationAttribute)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is className
defined?
code/nxsOnto/nxsOnto/generator.py
Outdated
@@ -184,12 +175,12 @@ def addFieldToDict(classDict, field, defn_name): # make a function to be reused | |||
#print('~~~ field did not exist: %s' % long_name) | |||
classDict[className]['fields'][long_name] = {} # create dictionary for field if doesn't exist |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicer to get the sub-dictionary classDict[className]['fields']
once rather than using the same copy and pasted expression. This applies for all dictionary access below too.
code/nxsOnto/nxsOnto/generator.py
Outdated
class NeXusClass(AnnotationProperty): | ||
pass | ||
|
||
|
||
class unitCategory(NeXus): | ||
comment = 'NeXus unit category. Can be considered instances of a measure. Assign data properties ' 'hasValue(any), hasMinValue(any), hasMaxValue(any), hasUnits(str)' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reformat this line
code/nxsOnto/nxsOnto/generator.py
Outdated
g = Github(token) | ||
repo = g.get_repo(nexus_repo) | ||
|
||
with urllib.request.urlopen(repo.tags_url) as url: | ||
tags = json.loads(url.read().decode()) | ||
tagsDict = tags[0] # get version tags from master branch | ||
|
||
|
||
base_class_url = [] | ||
for file in repo.get_contents("base_classes"): | ||
if str(file).split('.')[-2] == 'nxdl': | ||
base_class_url += [file.download_url] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use append
deprecationAttribute = field.getAttribute('deprecated') | ||
if not deprecationAttribute == '': | ||
print("=== Deprecation warning %s in %s: %s" % (field_name, className, deprecationAttribute)) | ||
print("=== Deprecation warning %s in %s: %s" % (field_name, className, deprecationAttribute)) | ||
|
||
long_name = className + join_string + field_name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use .join
code/nxsOnto/nxsOnto/generator.py
Outdated
import json | ||
import xml.dom.minidom | ||
import pickle | ||
from owlready2 import * |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not use wildcard imports
code/nxsOnto/nxsOnto/generator.py
Outdated
# create individuals - these are just for testing | ||
|
||
|
||
with onto: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test code should be in a separate function
#token = '' # insert your github token #out_path = 'ontology' #tmp_file_path = 'tmp'
Comment: #3 (comment)
Comment: #3 (comment)
Following the instructions in the README, I setup the conda environments and ran this code as instructed. Worked for me. Output text file was readable: (NeXusOntology) prjemian@zap:~/.../code/nxsOnto$ ll /tmp/onto/
total 904K
-rw-rw-r-- 1 prjemian prjemian 904K Dec 15 09:30 NeXusOntology.owl Aside from this PR: Must add the font size of the Protege software running on Windows with a 4k monitor is much too small. Even setting the preference for larger font, still there is tiny text in the window corners that is unreadable. Not an issue for this PR to resolve. |
example: (NeXusOntology) prjemian@zap:/tmp/onto$ python -m nxsOnto.generator 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 /tmp/onto /tmp
Deprecation warning definition_local in NXentry: see same field in :ref:`NXsubentry` for preferred use
Deprecation warning average_value_error in NXlog: see: https://github.com/nexusformat/definitions/issues/639
Deprecation warning wavelength_error in NXmonochromator: see https://github.com/nexusformat/definitions/issues/820
Deprecation warning energy_error in NXmonochromator: see https://github.com/nexusformat/definitions/issues/820
Deprecation warning radiation in NXsource: Use either (or both) ``probe`` or ``type`` fields from ``NXsource`` (issue #765)
Deprecation warning incident_wavelength_weight in NXbeam: use incident_wavelength_weights, see https://github.com/nexusformat/definitions/issues/837 |
I would like to see these commands: NeXusOntology/code/nxsOnto/nxsOnto/generator.py Lines 449 to 454 in 2ccdd90
wrapped into a function such as: def main():
dictionary_from_types()
dictionary_from_base_class_files()
parse_base_classes()
parse_application_definitions()
write_ontology()
create_test_individuals() then add this code at the end of the file: if __name__ == "__main__":
main() These changes enable nexusontology [token] [output_dir] [temporary_dir] With the changes above, here is my suggested revised #!/usr/bin/env python
#
# To use this file type:
#
# python setup.py install
from setuptools import setup
__entry_points__ = {
"console_scripts": [
"nexusontology = nxsOnto.generator:main",
],
# 'gui_scripts': [],
}
setup(name='NeXusOntologyGenerator',
version='1.1',
description='Generates an ontology from nxdl files',
author='Steve Collins',
url="https://github.com/nexusformat/NeXusOntology/code/nxsOnto",
packages=['nxsOnto'],
license='Apache 2',
entry_points=__entry_points__,
) |
Can the branches be merged? Or are there some objections? |