Add subscriptable support for PropertyHolder

[dvaerum]

This would be a simple, nice to have feature for dealing with custom fields 😃

It makes it possible to do

some_custom_field = "customfield_15623"
value = issue.fields[some_custom_field]

instead of

some_custom_field = "customfield_15623"
value = getattr(issue.fields, some_custom_field)

Alternatively, I will just keep patching it in

from jira import resources as jira_resources

def _jira_issue_property_holder_get_item(cls, item):
    if item.startswith("_"):
        raise KeyError(item)
    else:
        return getattr(cls, item)

jira_resources.PropertyHolder._getitem__ = _jira_issue_property_holder_get_item

[studioj]

sounds cool... a solution to a problem we all encountered, i cant say if this is an ideal solution on the other hand :-s seems to be open for much subjectiveness ... I would prefer some other people to have a look before approving.

in any case, please extend the tests. We wont merge anything without a proper test.

Also you have a mypy error

[dvaerum]

in any case, please extend the tests. We wont merge anything without a proper test.

I will try to have a look next week at work, to see if I can create some proper test for review, pointers are welcome 😉

[studioj]

@adehad your 2 cents on this would be appreciated. As said it's a possible solution to a problem many people are having. (Including me)
It doesn't make it auto completable on the other hand... Which is something we're trying to aim for with the mypy typing etc...
So I'm not 100%sure this is THE solution we should opt for.
On the other hand it's a definitely an improvement for useability.
I was thinking to do something with the actual name of the field. Instead of this custom_blabla thing. I'm guessing that's actually what @dvaerum wants to fix in the end.

First look up the custom field number then get the atr since you can only get the string from JIRA().fields()...

[adehad]

It doesn't make it auto completable on the other hand... Which is something we're trying to aim for with the mypy typing etc...

I think auto-completion/suggestions won't be possible just yet with how mypy and typing work. I'd be gladly proved wrong, but I think let's not worry about that for now.

I was thinking to do something with the actual name of the field. Instead of this custom_blabla thing. I'm guessing that's actually what @dvaerum wants to fix in the end.

First look up the custom field number then get the atr since you can only get the string from JIRA().fields()...

So we currently have _fields set in the __init__ which will translate the custom field name to the id.

jira/jira/client.py

Lines 497 to 501 in f26ea39

	self._fields = {}
	for f in self.fields():
	if "clauseNames" in f:
	for name in f["clauseNames"]:
	self._fields[name] = f["id"]

Which is used in search_issues:

jira/jira/client.py

Lines 2722 to 2727 in f26ea39

	untranslate = {} # use to add friendly aliases when we get the results back
	if self._fields:
	for i, field in enumerate(fields):
	if field in self._fields:
	untranslate[self._fields[field]] = fields[i]
	fields[i] = self._fields[field]

So perhaps we could look into creating a function that goes from field name to id, although I have a feeling that there may be cases where custom fields with the same name can exist - so I personally wouldn't want to rely on a solution like that in my work, but no harm documenting this limitation in such a function.

@adehad your 2 cents on this would be appreciated. As said it's a possible solution to a problem many people are having. (Including me)

Apart from __getattr__ raising a KeyError (it should raise an AttributeError really), my only concern is what this would do to other non-Issue objects. If we are able to verify with some test cases that this is fine then I'm happy with this approach.

What I've done in my work is actually something like:

some_custom_field = "customfield_15623"
value = issue.raw["fields"][some_custom_field]

From my perspective the most safe way to implement this would be so add directly to the Issue class something like a get_field() function:

    def get_field(self, field_name):
        return self.raw["fields"].get(field_name)

[dvaerum]

From my perspective the most safe way to implement this would be so add directly to the Issue class something like a get_field() function:

    def get_field(self, field_name):
        return self.raw["fields"].get(field_name)

I see what you @adehad with a method being a more preferred way to go, but I don't like that it will return None if the field does not exist because I feel that one could easily run into some small problems debugging to find a typo this way, instead of just right away being told that something is wrong

It doesn't make it auto completable on the other hand... Which is something we're trying to aim for with the mypy typing etc...

I think auto-completion/suggestions won't be possible just yet with how mypy and typing work. I'd be gladly proved wrong, but I think let's not worry about that for now.

Regarding auto-complete, I also don't know how one could do this, but I did get an idea there could help a little (inspired by @adehad preference suggestion)

def get_field(self, field_name: str) -> Any:
    ...

def get_field_int(self, field_name: str) -> int:
    ...

def get_field_str(self, field_name: str) -> str:
    ...

def get_field_dict(self, field_name: str) -> Dict[str, Any]:
    ...

def get_field_list(self, field_name: str) -> List[Any]:
    ...

I just don't know if this is something you wanted, or if it becomes too complete, but my logic is that if you want to get/use a customfield, you already know what type it is, so this way you would get (some) auto-completion for whatever variable is returned

[adehad]

I see what you @adehad with a method being a more preferred way to go, but I don't like that it will return None if the field does not exist because I feel that one could easily run into some small problems debugging to find a typo this way, instead of just right away being told that something is wrong

Yep I was just copying the behaviour of getattr() in your original example, I don't mind using square brackets access.

I just don't know if this is something you wanted, or if it becomes too complete, but my logic is that if you want to get/use a customfield, you already know what type it is, so this way you would get (some) auto-completion for whatever variable is returned

I think for now we can encourage a use like this for the type suggestion:

def get_field(self, field_name: str) -> Any:
    ...

# user to specify specific types
my_int: int = issue.get_field("customfield_15623")
my_str: str = issue.get_field("customfield_15623")

I was thinking @studioj might have been referring to autocompletion in the editor, e.g.

where currently we have listed some fields that should exist in the _IssueFields object to allow some basic autocompletion and type support. Which I think would be difficult to extend for custom fields as they vary between users.

[adehad]

Some works needs to be done on the tests and documentation, but I think this approach is shaping up well

[studioj]

tests still need some rework

just forget your changes in all the other tests as @adehad mentions

just add two testmethods
one where you raise an arrtibute error on purpose

self.assertRaises(AttributeError, get_field, "_name_starts_with_underscore")
and another one which follows the happy path

[rynkk]

Hey, any updates on this?
I am currently stuck with dynamically manipulating the __class__ of the _IssueFields-instance to include my very professional Mixin:

GetItemMixin

 class GetItemMixin:
    def __getitem__(self, item: str):
        try:
            return getattr(self, item)
        except AttributeError:
            raise ValueError(f"Field {item} does not exist in fields, is it included in the fields parameter?")

    def __setitem__(self, key: str, value: Any):
        if key in self and self[key] is not None and value is not None:
            assert isinstance(value, type(self[key]))  # some help with setting the correct value kind
        setattr(self, key, value)

    def __contains__(self, item):
        try:
            getattr(self, item)
            return True
        except AttributeError:
            return False

[dvaerum]

Hey, any updates on this? I am currently stuck with dynamically manipulating the __class__ of the _IssueFields-instance to include my very professional Mixin:
GetItemMixin

I ended up getting quite busy until new-year stuff is starting to calm down again. I may be able to have a look at it again because I also need it myself at work. Cannot keep patching this feature in, in multiple scripts 😅

[dvaerum]

@adehad

I have updated the patch. It should now contain the documentation you suggested, I have also, created an unittest there tests success and 2 different exceptions. If there are scenarios that I overlooked, plz tell me and I will get them added 🙂

[adehad]

Awesome, looking much better @dvaerum.

Just a minor point regarding the naming convention of variables.

There is also a non-blocking comment, so feel free to ignore if you feel strongly about it.

[dvaerum]

Awesome, looking much better @dvaerum.

Just a minor point regarding the naming convention of variables.

There is also a non-blocking comment, so feel free to ignore if you feel strongly about it.

Done, I have added all your suggestions (and comments to them), but it doesn't look like it will run the tests :/

[adehad]

Awesome, thanks for getting this through!

[dvaerum]

Awesome, thanks for getting this through!

Thanks for working with me on this :D