Resolve "Migration guide pyotb 1.5.4 --> 2.0.0"

Closes #104 (closed)

I took the liberty to add OTBObject.summarize() for backward compatibility purpose, and also convenience (this way the API is more flexible). However I am open to discuss its non-return, not a big deal.

Basically this MR consists in new decorators (in deprecated.py) for the depreciation of methods, arguments, and the otbObject class. Flashy color warning appear to suggest user to replace arguments names, when it's backward compatible, and exception are thrown (DepreciationWarning) when it's not. I have brought back the App.__getattr__ only to throw a DepreciationWarning explaining how to migrate, when the App.app attribute exists.

Maybe I have missed a few old attributes. With the decorators we can add them for depreciation easily.

to do

• add warnings and depreciation decorators in code
• write a note on migration in documentation

added docs label

changed the description

I get it but I'm a bit sceptical, almost 200 new lines of code and a new module, just to avoid them to read the docs, re-implementing a whole deprecation logic...if it was just me, either I'll wait the deprecation to be well supported, either I'll let them find by themselves that they must update their code and read the docs.

But I find this MR quite big just to deprecate 6 items.

Especially I don't like the coming back of __getattr__ which comes with instability and unexpected results, this is why we removed it, so I don't want to keep this in the code up to the 3.0 release

Yes this code is temporary and we can decide to remove in pyotb 3 for instance.

We then will just have to do the following, which is straightforward:

• remove deprecated module, hence:
  • remove depreciation decorators in core
    • remove deprecated methods
  • remove depreciation warnings in core, hence:
    • remove App.__getattr__
    • remove otbObject stub

Very simple since when you remove the depreciation module, everything follows in fall.

just to avoid them to read the docs

This is the whole point. It's a very common practice to depreciate stuff when public API has breaking changes. 200 LOC for that is nothing, in addition it is designed to be removed quickly. It's important to keep on track people that already use pyotb.

I don't like the coming back of __getattr__ which comes with instability and unexpected results

I have modified the __getattr__ method, see the diff

If you take a look in the code, nothing unexpected should happen. (edit: we can't be sure of that)

• getattr(self.app, item) is called, this is just to check that self.app has some item attribute. If not, we let the genuine exception be thrown. In the end, an exception for depreciation is always thrown.
• remember that getattr() is called only when the attribute has not been found. Since we don't do opaque things like setattr or playing with __dict__ like in 1.5.4, getattr() should not induce unexpected results or instability

That being said I might miss things, and to be 100% sure that no strange behavior pops back due to getattr() implementation. If you prefer, we could get rid of the call to getattr(self.app, item), and instead only check for item.startswith() to try to give a hint to the developer (which was the purpose of this part of code).

Something like:

# removed snippet, see the diff

Do you think that would still cause potential problems? maybe this exception is not the expected one from the outside...

or maybe just

    def __getattr__(self, item: str):
        note = "Since pyotb 2.0.0, OTBObject instances have stopped to "
               "forward attributes to their own internal otbApplication "
               "instance. `App.app` can be used to call otbApplications "
               f"methods. Attribute was: \"{item}\"."

        if item[0].isupper():
            # Because otbApplication instances methods names start with an upper case
            note += f"maybe try `pyotb_app.app.{item}` instead of `pyotb_app.{item}`?"

        if item.startswith("GetParameter"):
            note += (
                " Note: `pyotb_app.app.GetParameterValue('paramname')` can be "
                "shorten with `pyotb_app['paramname']` to access parameters "
                "values."
            )
        warning_msg(note)
        raise AttributeError

This should not cause any harm

added 1 commit

• 64d2dc7d - WIP: depreciation

Compare with previous version

added 1 commit

• 240ce351 - WIP: depreciation

Compare with previous version

requested review from @vidlb

added 1 commit

• 82c93bdb - REFAC: rename warning_msg --> depreciation_warning

Compare with previous version

changed milestone to %Release 2.0

resolved all threads

Sorry I tried with suggestion but when changes are nested it's kind of a mess, may be I should have pushed a commit.
All right I fixed my mess (merged several suggestions into one).

I think it's OK for me with these suggestions, except for the otbObject question.

added 1 commit

• e4d9a3e4 - Apply 1 suggestion(s) to 1 file(s)

Compare with previous version

added 1 commit

• 0e49fec8 - Apply 1 suggestion(s) to 1 file(s)

Compare with previous version

added 3 commits

• 8b8389aa - DOC: fix summarize api doc
• 69fce989 - Merge branch '104-migration' of gitlab.orfeo-toolbox.org:nicolasnn/pyotb into 104-migration
• cabb7f0a - FIX: AttributeError message

Compare with previous version

added 1 commit

• 62bc96a9 - FIX: AttributeError message

Compare with previous version

added 1 commit

• 56dea1ac - FIX: remove otbObject

Compare with previous version

changed the description

resolved all threads

approved this merge request

@remicress may be you should fix this 2d checklist item in !91 (merged) instead to avoid merge conflicts. I think this one is ready, I did not try all those attributes but it looks good

added 1 commit

• 3bedecfc - STYLE: run black

Compare with previous version

added 1 commit

• 0d7dc0c4 - STYLE: run black on core.py

Compare with previous version

You were just missing the black format. Which IDE are you using ?
It should detect the pyproject.toml config. If black is installed you can just right click to autoformat.

added 1 commit

• 9d27acfa - Apply 1 suggestion(s) to 1 file(s)

Compare with previous version

resolved all threads

added 28 commits

• 9d27acfa...77af1ded - 25 commits from branch develop
• e58d0fb3 - Merge branch 'develop' of gitlab.orfeo-toolbox.org:nicolasnn/pyotb into 104-migration
• b5a3c809 - DOC: add a note for migration in troubleshooting
• 1be1f2b6 - Merge branch '104-migration' of gitlab.orfeo-toolbox.org:nicolasnn/pyotb into 104-migration

Compare with previous version

added 1 commit

• 19d682d8 - DOC: add a note for migration in troubleshooting

Compare with previous version

marked the checklist item write a note on migration in documentation as completed

merged

mentioned in commit 02fa968d