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
Merged

Conversation

@lrosenth
Copy link
Contributor

@lrosenth lrosenth commented Feb 15, 2021

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 requested review from subotic and BalduinLandolt 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

@BalduinLandolt BalduinLandolt commented Feb 16, 2021

@lrosenth please add knora/.DS_Storeto .gitignore

Loading

Copy link
Collaborator

@BalduinLandolt BalduinLandolt left a comment

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)

Loading

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

@subotic subotic left a comment

  • 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.

Loading

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

@subotic subotic left a comment

great!

Loading

Copy link
Collaborator

@BalduinLandolt BalduinLandolt left a comment

Looks good to me

Loading

@lrosenth lrosenth merged commit 3628992 into main Feb 17, 2021
1 check passed
Loading
@lrosenth lrosenth deleted the wip/DSP-1341-lists-from-excel branch Feb 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants