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

Use standalone result library instead #494

Closed
Fuyukai opened this Issue Apr 14, 2018 · 5 comments

Comments

Projects
None yet
3 participants
@Fuyukai
Contributor

Fuyukai commented Apr 14, 2018

As said in #477, the Result objects got split out into a separate library (https://github.com/python-trio/outcome). There's no point duplicating all the code in Trio - better to depend on it directly.

Todo:

  • Gut result code inside of Trio
  • Use the results from outcome instead.

Maybe make the current objects in Trio simply redirect to the outcome objects until the next version, deprecating them in 0.5 (with a redirect) and removing in 0.6.

@njsmith

This comment has been minimized.

Member

njsmith commented Apr 14, 2018

CC @RazerM

There may be a small bit of awkwardness here because outcome also tweaked the API: Result.capture got replaced with outcome.capture instead of outcome.Outcome.capture. This is probably a good change on its own merits (the only reason I stuck capture onto the Result type was because I didn't have a proper module to put it on), but it means we can't just alias Result to outcome.Outcome and have everything work. The simplest thing might be to do:

class Result(Outcome):
    @classmethod
    def capture(...):
        ...

    @classmethod(...)
    async def acapture(...):
        ...

Result.register(Outcome)  # yes, they're subclasses of each other, what's wrong with that?

Gross hacks are fine because it's just a temporary shim that we'll throw away in a few versions after the deprecation period...

@RazerM

This comment has been minimized.

Member

RazerM commented Apr 14, 2018

@njsmith

This comment has been minimized.

Member

njsmith commented Apr 15, 2018

Bah, fine. I bet CPython isn't any fun at parties either.

In this case I think it would be enough to register just the concrete classes outcome.Value and outcome.Error.

@Fuyukai

This comment has been minimized.

Contributor

Fuyukai commented Apr 15, 2018

The issue with this is that I cannot see a way to meet all of the following:

  • Keep the same API (i.e. capture/acapture) for all types
  • Make isinstance() work properly with both outcome and _core._result types

If the first point is lifted, it can simply be Value = outcome.Value / Error = outcome.Error and register them as virtual subclasses of Result.

@njsmith

This comment has been minimized.

Member

njsmith commented Apr 15, 2018

Oh, you mean because right now someone could write Value.capture(...), and that would break if we do Value = outcome.Value? I guess that's technically true, but I doubt anyone's ever done that and I'm fine with just ignoring it. So I think I'm agreeing with your suggestion.

njsmith added a commit to Fuyukai/trio that referenced this issue Apr 21, 2018

Fix __deprecated_attributes__ usage
- The dict needs to live in the module where the deprecated attributes
  are; so here it needs to be hazmat.__deprecated_attributes__.
  (Maybe it would be less confusing to put this in hazmat.py, I don't
  know.)

- Don't export the symbols from trio.hazmat using normal mechanisms;
  that overrides the __deprecated_attributes__ mechanism.

- Then since they're no longer in hazmat, we can't say that
  trio.hazmat.Value resolves to the value trio.hazmat.Value, so point
  the __deprecated_attributes__ entries directly at trio._core._result.*

- In fact, let's stop exporting them from trio._core entirely.

- And then this reveals two tests that were still referring to
  trio._core.Error, so fix those.

Confirmed manually that with these changes I get correct values and
warnings for accessing the public attributes:

In [2]: trio.hazmat.Result
/home/njs/.user-python3.5-64bit/bin/ipython:1: TrioDeprecationWarning: trio.hazmat.Result is deprecated since Trio 0.5.0; use outcome.Outcome instead (python-trio#494)
  #!/home/njs/.user-python3.5-64bit/bin/python3.5
Out[2]: trio._core._result.Result  # class

In [3]: trio.hazmat.Value
/home/njs/.user-python3.5-64bit/bin/ipython:1: TrioDeprecationWarning: trio.hazmat.Value is deprecated since Trio 0.5.0; use outcome.Value instead (python-trio#494)
  #!/home/njs/.user-python3.5-64bit/bin/python3.5
Out[3]: outcome.Value  # class

In [4]: trio.hazmat.Error
/home/njs/.user-python3.5-64bit/bin/ipython:1: TrioDeprecationWarning: trio.hazmat.Error is deprecated since Trio 0.5.0; use outcome.Error instead (python-trio#494)
  #!/home/njs/.user-python3.5-64bit/bin/python3.5
Out[4]: outcome.Error  # class

@Fuyukai Fuyukai closed this in #503 Apr 21, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment