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: import lists from excel (DSP-1341) #48

Merged
merged 40 commits into from Feb 17, 2021

Conversation

lrosenth
Copy link
Contributor

Added support for hierarchical lists in excel files and updated the documentation to make it more consistent and a bit easier to navigate.

@lrosenth lrosenth added the enhancement New feature or request label Feb 15, 2021
@lrosenth lrosenth self-assigned this Feb 15, 2021
@subotic subotic changed the title Wip/dsp 1341 lists from excel feat: import lists from excel (DSP-1341) Feb 16, 2021
@BalduinLandolt
Copy link
Collaborator

@lrosenth please add knora/.DS_Storeto .gitignore

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.

See my review comments.

Also note the following things I could not comment directly into the files:

  • please add .DS_Store to .gitignore
  • please ensure not to include any sensitive data in CSVs
  • be sure not to include any copyright protected files as test data (I haven't checked images and audio, but PDF is an NZZ article which is not public domain)

knora/bitstreams/test.csv Show resolved Hide resolved
knora/onto-visualize.py Outdated Show resolved Hide resolved
knora/tmp_xlsx2list.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@subotic subotic left a comment

Choose a reason for hiding this comment

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

  • in general BUILD files should be named BUILD.bazel
  • could you please clean-up a bit the /knora folder and move the test data to another directory. It has become very messy. Only data that needs to be shipped, should be part of the /knora folder.

docs/index.md Show resolved Hide resolved
Copy link
Collaborator

@subotic subotic left a comment

Choose a reason for hiding this comment

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

great!

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.

Looks good to me

@lrosenth lrosenth merged commit 3628992 into main Feb 17, 2021
@lrosenth lrosenth deleted the wip/DSP-1341-lists-from-excel branch February 17, 2021 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants