Skip to content
This repository has been archived by the owner on Nov 17, 2022. It is now read-only.

Improve usage of version parameter #218

Open
5 tasks done
hayfield opened this issue Nov 7, 2017 · 6 comments
Open
5 tasks done

Improve usage of version parameter #218

hayfield opened this issue Nov 7, 2017 · 6 comments
Labels
api Changes to the pyIATI API. awaiting-deployment The changes are complete, in `dev`, and waiting to be merged to `master` and released. enhancement Some sort of new functionality (rather than fixing or tweaking something that already existed). versions Relating to IATI Version Numbers.

Comments

@hayfield
Copy link
Contributor

hayfield commented Nov 7, 2017

At the moment a number of functions take a version parameter to specify the version of IATI data to access. The current situation only allows specification of Decimal Versions. When unspecified, it defaults to the latest version. This has multiple problems.

  • It is unable to categorise values valid against all Decimal Versions within an Integer Version
  • It is unable to categorise data that is independent of any Version
  • The default value will lead to non-backwards-compatible return values upon an Integer Upgrade

As such, the following changes should be made:

  • Continue to allow specification of Decimal Versions (eg. 1.04, 1.05, 2.01) to return information from that specific version.
  • Allow specification of only the Integer Component of a Version Number (eg. 1, 2). This will return information that is applicable to all Decimal Versions within the Integer Version. If no data matches this, it will return information for the latest Decimal Version within the Integer Version.
  • Change the default parameter of None to mean version-independent (the folder name for relevant data).
  • Remove any power to assume the global latest version. iati.constants.STANDARD_VERSION_LATEST must be used instead.
  • Should an invalid Integer or Decimal Version be specified, or version-independent stated when not appropriate, a ValueError will be raised.
@hayfield hayfield added api Changes to the pyIATI API. enhancement Some sort of new functionality (rather than fixing or tweaking something that already existed). labels Nov 7, 2017
hayfield added a commit that referenced this issue Nov 7, 2017
This changes the back-end behaviour of the version argument, as required by #218.

Changes are only made to the resource function that looks at folder names. All other functionality using this is currently failing.
hayfield added a commit that referenced this issue Nov 7, 2017
In relation to #218

A lot of the changed code is fixed to 2.02 as this was the previous implementation. This will need changing.
@hayfield hayfield mentioned this issue Nov 7, 2017
15 tasks
@hayfield
Copy link
Contributor Author

hayfield commented Nov 15, 2017

This will return information that is applicable to all Decimal Versions within the Integer Version. If no data matches this, it will return information for the latest Decimal Version within the Integer Version.

This adds a level of complexity to usage since it may require a level of try-except on OSErrors. In some cases this can be worked around by using logic to preemptively determine which of the options should be followed because the other is impossible.

NOTE: The version argument is only present in functions in **/resources.py and default.py. These should be each act consistently within themselves, and have some amount of common approach where appropriate.

@hayfield
Copy link
Contributor Author

With version in resources.py...

  • A number of functions return arrays
  • At present, these arrays contain a single item
  • Upon implementation of SemVer, these will return multiple items

As such, a method needs to be determined to: guarantee that a single value will be returned into the future; or have a different API once SemVer appears.

The former of these options is preferable since it reduces the changes required in the future. A method that returns a list with a known order is not a suitable solution since when there is a single item there are multiple ways to access it (it's both the first and last item on the list). Requiring one of these over the other does not lead to an intuitive interface.

As such, a latest boolean flag is to be added. If set to True, the single latest [thing] within the specified range will be returned. Not a list that a single thing needs extracting from, but the thing.

For example:

  • get_all_activity_schema_paths('2.01') would under the current system return a list containing a single Activity Schema, while under SemVer would contain a list containing multiple.
  • get_all_activity_schema_paths('2.01', latest=True) would under the current system return a single Activity Schema, while under SemVer would do the same

@hayfield
Copy link
Contributor Author

Based on fleshing out the above in #258, I'd suggest that rather than being a flag, there should be separate helper functions that perform the action of returning a single [thing].

hayfield added a commit that referenced this issue Nov 29, 2017
At the moment, this returns a list with a single Ruleset.
It will do more as #218 is progressed.
hayfield added a commit that referenced this issue Nov 29, 2017
At the moment, this returns a list with a single Ruleset.
It will do more as #218 is progressed.
@hayfield
Copy link
Contributor Author

hayfield commented Dec 19, 2017

Decorators should probably be used to deal with how versions are processed by functions.

Doing it internally to functions isn't very DRY and leads to fairly complex and confusing code.

@hayfield
Copy link
Contributor Author

hayfield commented Feb 8, 2018

None should not mean version-independent.

If you make a constant iati.constant.ANY_VERSION = None, you need to then use is rather than == whenever you're checking whether something is equal to this value... which isn't entirely clear to the end user (and means that any code using this would need modifying if "any" (version independent) becomes part of an iati.Version in the future, or the value changes in some other way)

As such, the value should currently be a string with some value. '*' is suggested. This value may well change, so a constant should be created and used in instances where this magic value is required.

hayfield added a commit that referenced this issue Feb 28, 2018
This allows '==' to be used to check whether a value is this or not, rather than requiring use of 'is'.
By doing this, it gives more scope to change the value in the future.

A constant has been added to contain this value, and should be used if this meaning is required.

For original explanation for change, see:
    #218 (comment)
@hayfield hayfield added the versions Relating to IATI Version Numbers. label Feb 28, 2018
@hayfield
Copy link
Contributor Author

hayfield commented Mar 21, 2018

The Version class at present handles Minor versions of the Standard. It may be desired to extend this to allow it to represent Major Versions (as a whole) rather than requiring use of strings or integers for this purpose. As such, the following is not completely fully complete:

Allow specification of only the Integer Component of a Version Number (eg. 1, 2). This will return information that is applicable to all Decimal Versions within the Integer Version. If no data matches this, it will return information for the latest Decimal Version within the Integer Version.

Finalisation of this should be undertaken within the scope of #265.

@hayfield hayfield added the awaiting-deployment The changes are complete, in `dev`, and waiting to be merged to `master` and released. label Mar 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api Changes to the pyIATI API. awaiting-deployment The changes are complete, in `dev`, and waiting to be merged to `master` and released. enhancement Some sort of new functionality (rather than fixing or tweaking something that already existed). versions Relating to IATI Version Numbers.
Projects
None yet
Development

No branches or pull requests

1 participant