Skip to content

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.

'''

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


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


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?


if not long_name in classDict[className]['fields'].keys():
#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 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

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

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.

4 participants