Skip to content

Should we add read-only attributes (or prefix with underscore)? #634

@PGijsbers

Description

@PGijsbers

Currently our python objects tend to have many attributes that we only want modified by the API itself, e.g. all of the reference to ids. However, by not prefixing those attributes or making them read-only, we don't signal clearly to the users that they should not be tampered with.

At the same time (partially because of this?), we introduce a lot of extra work to test behavior which should only occur if the user uses the API in bad faith, e.g. manually overwriting ids.

PEP8 proposes that non-public attributes be prefixed with _. There is also the option of creating read-only properties like such:

class OpenMLRun:
    def __init__(self, run_id):
        self._run_id = run_id

    @property
    def run_id(self):
        return self._run_id

Disadvantages to prefixing without a property:

  • all old code with read and write operations breaks
  • need to access 'non-public' attributes for run_id (for the correct use of reading the value).

Disadvantages to prefixing with a property:

  • all old code with write operations breaks
  • verbose as our classes may have many of attributes which should be read-only for outside users

Disadvantages of leaving as is:

  • more prone to users accidentally writing fields that they should not do
  • maintaining unit tests to test behavior in case those mistakes are made

There is of course also the opportunity of leaving things as they are without proper unit tests.
This might give some weird local behavior when things are done that shouldn't be done, but server-side there shouldn't be any issues (after all, you should be able to throw anything at the REST API directly anyway, right?).

What are your thoughts?

I feel that we should move in a direction which avoids creating/maintaining unit tests for users deliberately playing with our internals.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions