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

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 August 9, 2021 15:37
@irinaschubert
Copy link
Author

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!

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.

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.

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

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.

@irinaschubert irinaschubert merged commit 06d071a into main Aug 10, 2021
@irinaschubert irinaschubert deleted the wip/DSP-1580-create-lists-from-excel branch August 10, 2021 12:51
@irinaschubert irinaschubert restored the wip/DSP-1580-create-lists-from-excel branch August 10, 2021 15: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