Skip to content

Conversation

@fliiiix
Copy link
Contributor

@fliiiix fliiiix commented Jul 6, 2024

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
3b27f53 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 🔝 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

@github-actions github-actions bot added c++ codegen Involving generating code from schema grpc python labels Jul 6, 2024
@anton-bobukh
Copy link
Collaborator

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?

@fliiiix
Copy link
Contributor Author

fliiiix commented Jul 8, 2024

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 😅 )

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
Copy link
Contributor Author

fliiiix commented Jul 11, 2024

@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 fliiiix force-pushed the bugfix/reexport-python-type-infos branch from 8b41104 to 7c06c55 Compare July 12, 2024 19:23
@github-actions github-actions bot removed the grpc label Jul 12, 2024
@fliiiix fliiiix force-pushed the bugfix/reexport-python-type-infos branch from 7c06c55 to 7d6aa5e Compare July 12, 2024 19:26
Importing the Base type for `--gen-object-api` in
`.pyi` files eg. `from namespace.Type import Type`
can lead to re-export issues depeding on how the
generated code is embeded.

To prevent this the import is removed since
the data Type is always defined just on top.
@fliiiix fliiiix force-pushed the bugfix/reexport-python-type-infos branch from 7d6aa5e to 2a1790e Compare July 13, 2024 08:33
@github-actions
Copy link
Contributor

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 github-actions bot added the stale label Jan 11, 2025
@fliiiix
Copy link
Contributor Author

fliiiix commented Jan 12, 2025

not-stale

@github-actions github-actions bot removed the stale label Jan 12, 2025
@fliiiix
Copy link
Contributor Author

fliiiix commented Jun 25, 2025

this is addressed in a new MR where i prevent the unneeded import #8625

@fliiiix fliiiix closed this Jun 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ codegen Involving generating code from schema python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants