flatbuffers icon indicating copy to clipboard operation
flatbuffers copied to clipboard

Bugfix/reexport python type infos

Open fliiiix opened this issue 1 year ago • 3 comments

I tried to use the new .pyi files and stumbled over the re-export problem. This change adds the classes in .pyi to __all__ to allow re-export.

Regenerated the grcp for python correctly i think that was missed with 3b27f5396e84de2a131863ecdc96415af65d003b and feels to me like an unwanted regression in 1. generating the file in a different location and 2. generate it by default with a . in the file name which makes it nearly impossible to import with Python

@anton-bobukh :top: Did i misunderstand something?

And my clang-format didn't agree with some things. @dbaileychess Is there a specific version which should be used?

clang-format --version
clang-format version 18.1.6

fliiiix avatar Jul 06 '24 14:07 fliiiix

Thanks for the fixes, @fliiiix!

Can you provide more details on the re-export issue? Why does the default behaviour – exporting everything that does not start with a _ – not work?

As for the dot in the filename, it looks like a bug. It should be an _ instead with the default being _fb, not .fb. Also, not sure about 1. generating the file in a different location, what do you mean by that?

anton-bobukh avatar Jul 08 '24 17:07 anton-bobukh

re-export issue

Can you provide more details on the re-export issue?

I will look more into that, i'm not a 100% sure yet how it works. I think it has something todo with the fact that the info is in a stub file. (But maybe we are doing something weird in our setup :sweat_smile: )

Note that mypy treats stub files as if this is always disabled. https://mypy.readthedocs.io/en/stable/config_file.html#confval-implicit_reexport

underscore issue

It should be an _ instead with the default being

Cool so i hopefully find time this week to provide a fix for that

grpc file location

generating the file in a different location

we have a portal file something like this:

namespace data;

/// Simple feedback for whether something worked or not
table SuccessReply {
  /// Set to true when Setting was successful
  success:bool;
}

/// Used when no data needs to be transferred
table Nil {
}

Which generates a folder data containing SuccessReply.py, Nil.py, ...

And before some change the data_grpc_fb.py was inside this data folder now i think it is just generated where ever flatc is run. (Right now i just 'fixed' that by moving the file back with cmake)

fliiiix avatar Jul 08 '24 19:07 fliiiix

@anton-bobukh I marked this MR as Draft and opened #8359 for point 2 and 3.

I need some more time to analyze the re-export problem. As far as I could tell the problem is the import in the .pyi files

from __future__ import annotations

import flatbuffers
import numpy as np

import flatbuffers
import typing
from portal_data.Nil import Nil # <- here this is i think considered re-exported under some conditions 

uoffset: typing.TypeAlias = flatbuffers.number_types.UOffsetTFlags.py_type

class Nil:
  ...
class NilT:
   ...

def Pack(self, builder: flatbuffers.Builder) -> None: ...
def NilStart(builder: flatbuffers.Builder) -> None: ...
def Start(builder: flatbuffers.Builder) -> None: ...
def NilEnd(builder: flatbuffers.Builder) -> uoffset: ...
def End(builder: flatbuffers.Builder) -> uoffset: ...

I'm not sure yet if the best approach is to remove the import or use this __all__ export or even do a from foo import bar as bar.

fliiiix avatar Jul 11 '24 15:07 fliiiix

This pull request is stale because it has been open 6 months with no activity. Please comment or label not-stale, or this will be closed in 14 days.

github-actions[bot] avatar Jan 11 '25 20:01 github-actions[bot]

not-stale

fliiiix avatar Jan 12 '25 09:01 fliiiix

this is addressed in a new MR where i prevent the unneeded import https://github.com/google/flatbuffers/pull/8625

fliiiix avatar Jun 25 '25 06:06 fliiiix