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

feat: add command excel2json to create JSON project file from folder with Excel files (DEV-960) #248

Merged
merged 10 commits into from Nov 9, 2022

Conversation

jnussbaum
Copy link
Collaborator

resolves DEV-960

@jnussbaum jnussbaum self-assigned this Nov 3, 2022
@@ -10,7 +10,7 @@ CURRENT_DIR := $(shell dirname $(realpath $(firstword $(MAKEFILE_LIST))))
.PHONY: dsp-stack
dsp-stack: ## clone the dsp-api git repository and run the dsp-stack
@mkdir -p .tmp
@git clone --branch main --single-branch --depth 1 https://github.com/dasch-swiss/dsp-api.git .tmp/dsp-stack
@git clone --branch v24.0.8 --single-branch https://github.com/dasch-swiss/dsp-api.git .tmp/dsp-stack
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is temporary, I plan to revert this as soon as the other PR is merged that fixes the project IRIs. But as long as the API is not released, I don't want to merge the other PR, so that I could also make a release of dsp-tools before the API is released.

Copy link
Collaborator

@BalduinLandolt BalduinLandolt left a comment

Choose a reason for hiding this comment

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

I don't fully understand the motivation for this PR, but in terms of code it looks good

Comment on lines -256 to +257
with open("knora/dsplib/schemas/lists-only.json") as schema:
current_dir = os.path.dirname(os.path.realpath(__file__))
with open(os.path.join(current_dir, "../schemas/lists-only.json")) as schema:
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this change really needed? seems more complex to me, without any added benefit I could think of

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The old version only worked because during development, I always invoke commands from the directory dsp-tools (=working directory). As soon as the working directory is different, it doesn't work any more. I wonder why it worked for our customers until now...

Copy link
Collaborator

Choose a reason for hiding this comment

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

did you install it with -e flag maybe? in any case, if that fixes it, all the better

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I thought that would be the only sensible way to install it for development. The point is that I often want to test a dsp-tools command from terminal, and I want that it takes the one I am editing/developing currently.

Copy link
Collaborator

Choose a reason for hiding this comment

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

sure, makes sense, but of course this may change the behaviour of relative paths compared to how users experience it

Comment on lines 20 to 26
data_model_templates
|-- lists
| |-- de.xlsx
| `-- en.xlsx
`-- onto_name (onto_label)
|-- properties.xlsx
`-- resources.xlsx
Copy link
Collaborator

Choose a reason for hiding this comment

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

this kind of setup doesn't seem very user friendly to me... but I suppose this was identified as a requirement?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the hint. I'm going to ask some RDU people what they think.

Comment on lines -23 to +26
with open("knora/dsplib/schemas/properties-only.json") as schema:
current_dir = os.path.dirname(os.path.realpath(__file__))
with open(os.path.join(current_dir, "../schemas/properties-only.json")) as schema:
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as above

Comment on lines -21 to +25
with open("knora/dsplib/schemas/resources-only.json") as schema:
current_dir = os.path.dirname(os.path.realpath(__file__))
with open(os.path.join(current_dir, "../schemas/resources-only.json")) as schema:
Copy link
Collaborator

Choose a reason for hiding this comment

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

as above

Comment on lines 7 to 28
import copy
import datetime
import json
import unittest
import os
import datetime
import re
import unittest

import jsonpath_ng
import jsonpath_ng.ext
import copy

from knora.dsplib.utils.excel_to_json_lists import excel2lists, validate_lists_section_with_schema
from knora.dsplib.utils.excel_to_json_project import excel2project
from knora.dsplib.utils.excel_to_json_properties import excel2properties
from knora.dsplib.utils.excel_to_json_resources import excel2resources
from knora.dsplib.utils.id_to_iri import id_to_iri
from knora.dsplib.utils.onto_create_ontology import create_project
from knora.dsplib.utils.onto_validate import validate_project
from knora.dsplib.utils.onto_create_lists import create_lists
from knora.dsplib.utils.onto_create_ontology import create_project
from knora.dsplib.utils.onto_get import get_ontology
from knora.dsplib.utils.xml_upload import xml_upload
from knora.dsplib.utils.onto_validate import validate_project
from knora.dsplib.utils.shared import validate_xml_against_schema
from knora.dsplib.utils.xml_upload import xml_upload
from knora.excel2xml import excel2xml
Copy link
Collaborator

Choose a reason for hiding this comment

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

what tool does this import sorting? would be nice for the diffs, if those would only show up once (i.e. always the same automation)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Some weeks ago, I told PyCharm to do this. I thought it would be a good idea. I only work with PyCharm, and I think I will continue doing so, so there is only 1 tool that does this sorting. Do you think it costs more nerves than it benefits?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, if it's only one tool and it does it in a consistent way, it's definitely a good thing

@jnussbaum jnussbaum changed the title feat: add command excel2project to create JSON project file from folder with Excel files (DEV-960) feat: add command excel2json to create JSON project file from folder with Excel files (DEV-960) Nov 7, 2022
@jnussbaum jnussbaum merged commit e8e05e4 into main Nov 9, 2022
5 checks passed
@jnussbaum jnussbaum deleted the wip/dev-960-feat-excel2project branch November 9, 2022 08:14
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.

None yet

2 participants