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.

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

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open

Create python package #3

wants to merge 23 commits into from

Conversation

hgoerzig
Copy link
Contributor

@hgoerzig hgoerzig commented Aug 18, 2021

  • 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

@PeterC-DLS
Copy link

Please edit this to provide a proper title and description

@hgoerzig hgoerzig changed the title Createpackage Create python package Aug 18, 2021
Copy link

@PeterC-DLS PeterC-DLS left a 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.

packages=['nxsOnto'],
license='Apache 2',
install_requires=[
"owlready2",

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

version='1.1',
description='Generates an ontology from nxdl files',
author='Steve Collins',
url="https://github.com/nexusformat/NeXusOntology/script",

Choose a reason for hiding this comment

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

Does not exist

Copy link
Contributor

Choose a reason for hiding this comment

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

@PeterC-DLS
Copy link

The README does not explicitly say how I run this and what the outcome is.

Copy link

@PeterC-DLS PeterC-DLS left a 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.

@@ -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

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.

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':

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 '.'?


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 == '':

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

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))

Choose a reason for hiding this comment

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

Where is className defined?

@@ -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

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.

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)'

Choose a reason for hiding this comment

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

Reformat this line

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]

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

Choose a reason for hiding this comment

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

Use .join

import json
import xml.dom.minidom
import pickle
from owlready2 import *

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

# create individuals - these are just for testing


with onto:

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

@prjemian
Copy link
Contributor

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.

@prjemian
Copy link
Contributor

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

@prjemian
Copy link
Contributor

I would like to see these commands:

dictionary_from_types()
dictionary_from_base_class_files()
parse_base_classes()
parse_application_definitions()
write_ontology()
create_test_individuals()

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 setup.py to describe an entry point so the code (as installed) can be started more easily, such as:

nexusontology [token] [output_dir] [temporary_dir]

With the changes above, here is my suggested revised setup.py:

#!/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__,
      )

@hgoerzig
Copy link
Contributor Author

Can the branches be merged? Or are there some objections?

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.

3 participants