Skip to content
This repository was archived by the owner on Jul 22, 2021. It is now read-only.

Conversation

@wkitka
Copy link
Member

@wkitka wkitka commented Jun 4, 2020

Make dsconfig compatible with Python 3.6 and drop support for Python 2.
Tested features: xls2json, json2tango, dsconfig.dump, dsconfig.viewer.

@ajoubertza
Copy link

@wkitka Thanks - we (SKA) were planning to do this long ago, but never got time to finish it.

We did add some tests as we wanted to increase the test coverage before migrating. Those are in a mirrored repo: https://gitlab.com/ska-telescope/lib-maxiv-dsconfig/-/tree/SAR-2/expand_test_coverage. You may want to include those, or at least try them out against your new code.

Note that some of the files are no longer needed - there is an open PR removing them: #12.

Would you mind filling in the description for this PR? For example, is this update dropping support for Python 2, or is it now compatible with both 2.7 and 3.6? Any limitations? Etc.

Copy link

@ajoubertza ajoubertza left a comment

Choose a reason for hiding this comment

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

I only looked at setup.py.

version="1.5.0",
packages=["dsconfig", "dsconfig.appending_dict"],
description="Library and utilities for Tango device configuration.",
# Requirements

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for comments! I'll improve this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

I included both mentioned PRs and added description

@bamartos
Copy link

bamartos commented Jul 1, 2020

Hej,

It looks also good from my side, shall we proceed to the merge?

@wkitka
Copy link
Member Author

wkitka commented Jul 1, 2020

Hej,

It looks also good from my side, shall we proceed to the merge?

Hej @bamartos
I think so. Thanks for checking!

Copy link

@ajoubertza ajoubertza left a comment

Choose a reason for hiding this comment

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

Thanks @wkitka
Fingers crossed (since this repo has no CI!) 🙂

@wkitka
Copy link
Member Author

wkitka commented Jul 1, 2020

@bamartos
would you do the honors and merge it?

@bamartos bamartos merged commit a31f67a into MaxIV-KitsControls:develop Aug 10, 2020
This was referenced Aug 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants