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

Adding auto generated code data type #213

Closed
wondie opened this issue Feb 7, 2017 · 15 comments
Closed

Adding auto generated code data type #213

wondie opened this issue Feb 7, 2017 · 15 comments
Assignees
Milestone

Comments

@wondie
Copy link
Contributor

wondie commented Feb 7, 2017

The code data type is useful to generate unique spatial unit code from administrative units. It will be great to integrate it.

@gkahiu
Copy link
Contributor

gkahiu commented Feb 8, 2017 via email

@wondie
Copy link
Contributor Author

wondie commented Feb 8, 2017

I think I can add it with the same implementation as Turkana parcel code for STDM 1.5 as the code already exists. But it can be re-designed and even use expression builder for STDM 2.0. What do you think?

@gkahiu
Copy link
Contributor

gkahiu commented Feb 8, 2017 via email

@wondie
Copy link
Contributor Author

wondie commented Feb 8, 2017

As it is column data type that should be chosen in the Column Editor, it should work for all entities. If there is more time, it can be customized to have what you have said. However, for 1.5, we can just focus of allowing the user to set administrative units creating the codes from the administrative unit codes and adding a serial number at the end. However, allowing users to decide on prefix and suffix of codes might require more time.

@gkahiu
Copy link
Contributor

gkahiu commented Feb 8, 2017

OK with me. @pgathogo comments?

@wondie
Copy link
Contributor Author

wondie commented Feb 9, 2017

The feature is included. However, considering the time and the use, I decided to add two properties.
When a user choose this column type, he can specify two options.
prefix_source
This could be a lookup table, None, or the admin_spatial_unit_set table. The behavior differs based on what is selected here.
If it is a lookup table, users will be given option to choose from the lookup values and codes. Then the code will be used as a prefix of the code.
eg. F/001, M/002
If it is None, there will be no prefix in the code. A serial number will be added in the column.
eg. 001, 002
If it is admin_spatial_unit_set, the prefix will be generated based on the admin hierarchy codes. If the user selects administrative unit level 3, then level 1 to level 3 code will be shown separated by forward slash.
eg. NRB/ROY/KAS/001, NRB/ROY/KAS/002

leading_zero (to be included later)
Refers to the number of zeros at the beginning. I am considering a combobox containing the following values. 0, 00, 000, 0000, 00000
If the user choose, 0, the code starts with 01, if the user chooses 000, the code starts with 0001. When the number reaches 100, the code will be 0100.

In the editor, the user needs to click on the button to generate a code for None prefix, and select a value/code in lookup columns and admin_spatial_unit_set prefix source.

In configuration.stc

    <Column unique="True" tip="" minimum="0" maximum="4000" description="" index="False" name="serial_code" searchable="True" TYPE_INFO="UNIQUE_CODE" mandatory="False">
     <code prefix_source="None"/>
    </Column>
    <Column unique="True" tip="" minimum="0" maximum="4000" description="" index="False" name="adm_code" searchable="True" TYPE_INFO="UNIQUE_CODE" mandatory="False">
     <code prefix_source="admin_spatial_unit_set"/>
    </Column>
    <Column unique="True" tip="" minimum="0" maximum="4000" description="" index="False" name="unique_lookup" searchable="True" TYPE_INFO="UNIQUE_CODE" mandatory="False">
     <code prefix_source="check_tenure_type"/>
    </Column>

Issue
Whenever I run the wizard, the setting is saved in configuration.stc and the current profile can read whatever is saved in the configuration.stc as shown below.

{'profile': <stdm.data.configuration.profile.Profile object at 0x000000001ADB0510>, 'index': False, 'unique': True, '_monitor_attrs': ['mandatory', 'searchable', 'index', 'unique'], 'name': u'serial_code', 'searchable': True, 'updated_db_attrs': {}, 'entity': <stdm.data.configuration.entity.Entity object at 0x000000001ADB0A60>, 'prefix_source': u'None', 'user_tip': u'', '_intialized': True, 'action': 1, 'mandatory': False, '_maximum': 4000, '_minimum': 0, 'description': u''}
{'profile': <stdm.data.configuration.profile.Profile object at 0x000000001ADB0510>, 'index': False, 'unique': True, '_monitor_attrs': ['mandatory', 'searchable', 'index', 'unique'], 'name': u'adm_code', 'searchable': True, 'updated_db_attrs': {}, 'entity': <stdm.data.configuration.entity.Entity object at 0x000000001ADB0A60>, 'prefix_source': u'admin_spatial_unit_set', 'user_tip': u'', '_intialized': True, 'action': 1, 'mandatory': False, '_maximum': 4000, '_minimum': 0, 'description': u''}
{'profile': <stdm.data.configuration.profile.Profile object at 0x000000001ADB0510>, 'index': False, 'unique': True, '_monitor_attrs': ['mandatory', 'searchable', 'index', 'unique'], 'name': u'unique_lookup', 'searchable': True, 'updated_db_attrs': {}, 'entity': <stdm.data.configuration.entity.Entity object at 0x000000001ADB0A60>, 'prefix_source': u'check_tenure_type', 'user_tip': u'', '_intialized': True, 'action': 1, 'mandatory': False, '_maximum': 4000, '_minimum': 0, 'description': u''}

But when I restart QGIS, the configuration object is not reading the prefix_source. It shows empty.

What am I missing?
Below is the data/configuration/columns.py code

class UniqueCodeColumn(BoundsColumn):
    """
    Enables the attachment of a unique identifier code to the entity.
    """
    TYPE_INFO = 'UNIQUE_CODE'
    SQL_MAX = 4000
    SQL_MIN = 0
    sql_updater = varchar_updater

    def __init__(self, *args, **kwargs):

        self.prefix_source = kwargs.pop('prefix_source', '')
        BoundsColumn.__init__(self, *args, **kwargs)

    def can_create_check_constraints(self):
        # No need to create constraints since the extents are set while
        # creating the column.
        return False

    @classmethod
    def display_name(cls):
        return tr('Unique Identifier Code')

UniqueCodeColumn.register()

config_serializer codes

class UniqueCodeColumnSerializer(VarCharColumnSerializer):
    """
    (De)serializes administrative spatial unit column type.
    """
    COLUMN_TYPE_INFO = 'UNIQUE_CODE'
    PREFIX_SOURCE = 'code'

    @classmethod
    def _convert_bounds_type(cls, value):
        return int(value)

    @classmethod
    def _write_xml(cls, column, column_element, document):
        # Append code prefix source
        dt_element = document.createElement(
            UniqueCodeColumnSerializer.PREFIX_SOURCE
        )
        dt_element.setAttribute('prefix_source', column.prefix_source)

        column_element.appendChild(dt_element)

@wondie wondie self-assigned this Feb 9, 2017
@wondie wondie added this to the STDM v1.5 milestone Feb 9, 2017
@gkahiu
Copy link
Contributor

gkahiu commented Feb 9, 2017

Hi Wondie,

This is a great feature! Quick question, is there an option of generating a code without the leading zeroes i.e. just plain 1, 2 ... or F/1, M1 ... or NY/OK/1, MK/8?

Regarding your issue, the code for deserializing the unique code from the file does not load information regarding the prefix source. You need to read the child element and extract the value from prefix_source attribute then add it into the list of optional arguments. Please have a look at the GeometryColumn here implementation where the geometry type, SRID and layer display name are extracted from the column child element and loaded into the constructor mandatory and optional arguments respectively.

A few other general comments regarding the code:

  • Can you change the class name to AutoGenerateColumn or something close? This also applies to the TYPE_INFO and display name. Other columns of VarChar type can also be unique so the naming might be a bit confusing.

  • Why did you not subclass it from VarCharColumn class since the generated values will be of character varying type and the updater is already defined. It would seem more logical to derive it from this class.

  • I would suggest you enable indexing by default as chances are high that this column might be used to search for features often.

  • The first letter for a tag name must be capitalized hence the c in '<code...' should be capitalized.

  • For any changes to an existing API, please remember to annotate it accordingly. In this case, there are new classes hence you need to indicate which version of STDM they were added in, see examples here and here. For more details, have a look at the syntax for Sphinx documentation.

Regards,

John

@wondie
Copy link
Contributor Author

wondie commented Feb 9, 2017

Hi John,

Thank you for the help and feedback.
Yes, we can add "None" so that there will be no leading zero.
I can also add separator options such as / - _ and None so that users can choose them.

Regarding the issue, the line you shared me doesn't show anything is pulled from the xml. It is setting values. I actually have tried that and still it is not getting values from the xml.

On the comments, I will work on all them. However, I have a comment for the display name of the column, do you think Auto Generate will be understandable for the user?

Regards,
Wondimagegn

@gkahiu
Copy link
Contributor

gkahiu commented Feb 9, 2017 via email

@wondie
Copy link
Contributor Author

wondie commented Feb 9, 2017 via email

@wondie wondie changed the title Adding spatial unit code data type Adding auto generated code data type Feb 9, 2017
@wondie
Copy link
Contributor Author

wondie commented Feb 10, 2017

This feature is now incorporated to master with this commit.

@wondie wondie closed this as completed Feb 10, 2017
@gkahiu gkahiu reopened this Feb 22, 2017
@gkahiu
Copy link
Contributor

gkahiu commented Feb 22, 2017

Ensure that the width of the combo boxes is uniform, see current situation now:
image

The icon for the editor looks strange, what does it depict? Could you use another image that depicts generate/make etc.?
image

I would suggest (in future) you refactor your code because the line edit also contains the code for generating the unique code, this breaks the SRP (Single Responsibility Principle). This means that one cannot use the logic for generating code in a different part of the application since its embedded within the UI. Also, this makes testing (through unit tests) the code for generating code much more difficult since it is embedded within another class. I would suggest you adopt a strategy pattern with pseudo code as follows:

class AbstractCodeGen(object):
    def generate(self):
        raise NotImplementedError

class DefaultCodeGen(AbstractCodeGen):
    def generate(self):
        return 'Default Code'

class WebGeneratedCode(AbstractCodeGen):
   def generate(self):
        return 'Server generated code'

class CodeLineEdit(QLineEdit):
    def __init__(self, code_gen=DefaultCodeGen):
        self.generator = code_gen()
   def generate_code(self):
        self.generator.generate()

Basically in a strategy pattern, there is separation of concerns and you can dynamically incorporate any type of generator in the line edit UI class as longs as it inherits AbstractCodeGen class for example. In addition, you can also write separate unit tests to test the code generation logic.

@wondie
Copy link
Contributor Author

wondie commented Feb 22, 2017

Thank you for the feedback. I will work on the combobox.

The icon represent bar code. I couldn't get any other good alternative. The best I could get is the following.
image
Keyword such as make, generate shows gear and tool icons.

Regarding the class, I just followed what was implemented for Administrative Unit and Related Entity Line edit already. I get your suggestion to separate the code Generation function in a different class. I will fix that in the future.

@wondie
Copy link
Contributor Author

wondie commented Feb 22, 2017

I have fixed the combobox size in this commit.

@wondie
Copy link
Contributor Author

wondie commented Mar 6, 2017

All issues are fixed now. Until issues are found, I will close it.

@wondie wondie closed this as completed Mar 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants