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(excel-lists): create multilanguage json lists from excel files (DSP-1580) #75

Merged
merged 16 commits into from Aug 10, 2021

Conversation

@irinaschubert
Copy link
Collaborator

@irinaschubert irinaschubert commented Jul 27, 2021

resolves DSP-1580

@irinaschubert irinaschubert changed the title add docstring to main feat(excel-lists): create multilanguage json lists from excel files (DSP-1580) Jul 27, 2021
@irinaschubert irinaschubert self-assigned this Jul 29, 2021
@irinaschubert irinaschubert marked this pull request as ready for review Aug 9, 2021
@irinaschubert
Copy link
Collaborator Author

@irinaschubert irinaschubert commented Aug 9, 2021

Hallo @BalduinLandolt , nicht erschrecken, das ist ein riesiger PR. Ich habe v.a. die Funktionalität eingebaut, mehrsprachige Listen aus Excel-Files zu erstellen. Dazu musste ich eine Menge weiteren Code anpassen und habe bei dieser Gelegenheit aufgeräumt. Das Ganze ist mir etwas aus dem Ruder gelaufen, ich habe viel mehr angepasst, als für die Excel-Funktionalität nötig gewesen wäre. Und ich könnte noch lange weiter machen. Aber ich möchte diesen Task natürlich auch bald einmal abschliessen, damit die Funktionalität dem CS-Team zur Verfügung steht.

Jedenfalls, der Kern der Sache liegt im File knora/dsplib/utils/excel_to_json_lists.py. Du hast den fast gleichen Code ja auch schon in dsp-tools-prep reviewed.

Vielen Dank schon mal!

Loading

Copy link
Collaborator

@BalduinLandolt BalduinLandolt left a comment

Some minor comments, apart from that I think it's a big improvement.

I think many of the things I noted are due to PyCharm's auto formtatting, which I'm not overly fond of... but that's cosmetics, so you can happily ignore those comments.

Loading

docs/dsp-tools-create.md Outdated Show resolved Hide resolved
Loading
docs/dsp-tools-excel.md Outdated Show resolved Hide resolved
Loading
docs/dsp-tools-excel.md Outdated Show resolved Hide resolved
Loading
knora/dsp_tools.py Show resolved Hide resolved
Loading
knora/dsplib/models/permission.py Show resolved Hide resolved
Loading
knora/dsplib/utils/knora-schema-lists-only.json Outdated Show resolved Hide resolved
Loading
knora/dsplib/utils/onto_create_lists.py Show resolved Hide resolved
Loading
knora/dsplib/utils/onto_create_ontology.py Outdated Show resolved Hide resolved
Loading
knora/dsplib/utils/onto_create_ontology.py Outdated Show resolved Hide resolved
Loading
knora/dsplib/utils/xml_upload.py Outdated Show resolved Hide resolved
Loading
@irinaschubert
Copy link
Collaborator Author

@irinaschubert irinaschubert commented Aug 10, 2021

Some minor comments, apart from that I think it's a big improvement.

I think many of the things I noted are due to PyCharm's auto formtatting, which I'm not overly fond of... but that's cosmetics, so you can happily ignore those comments.

I agree with everything and updated the files accordingly. I also changed some of my options in PyCharm in order to reformat the files nicely in the future.

Loading

@irinaschubert irinaschubert merged commit 06d071a into main Aug 10, 2021
2 checks passed
Loading
@irinaschubert irinaschubert deleted the wip/DSP-1580-create-lists-from-excel branch Aug 10, 2021
@irinaschubert irinaschubert restored the wip/DSP-1580-create-lists-from-excel branch Aug 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants