Skip to content
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

Docs: Import/Export references a function with a deprecation warning #682

Closed
DeflateAwning opened this issue Sep 2, 2024 · 6 comments
Closed
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@DeflateAwning
Copy link

In the Import/Export docs page, the very first example uses a deprecated function, as warned by Python linters.

The example in question:

with BuildPart() as box_builder:
    Box(1, 1, 1)
box_builder.part.export_step("box.step")

The warning:

The method "export_stl" in class "Shape" is deprecated
  Use the `export_stl` function instead
Pylance

I'm not sure if the issue is that the example shows the wrong method, or if the deprecation warning shouldn't be there, but it seems there's an issue.

@gumyr gumyr added the documentation Improvements or additions to documentation label Sep 2, 2024
@gumyr gumyr added this to the Not Gating Release 1.0.0 milestone Sep 2, 2024
@jdegenstein jdegenstein self-assigned this Sep 3, 2024
@jdegenstein
Copy link
Collaborator

Thanks for pointing it out -- the docs need to be updated.

@DeflateAwning
Copy link
Author

What is the correct way? I could probably submit a quick PR for this :)

@jdegenstein
Copy link
Collaborator

jdegenstein commented Sep 3, 2024

@DeflateAwning here is the correct syntax:

with BuildPart() as box_builder:
    Box(1, 1, 1)
export_step(box_builder.part, "box.step")

A PR is not necessary as I already added it to my draft docfix PR here #667

@DeflateAwning
Copy link
Author

Out of curiosity, why is this better? This seems less easy to find, and at the very least, doesn't seem like it needs to have a deprecation associated with the old way

@jdegenstein
Copy link
Collaborator

It is deprecated because we are planning to remove it, just trying to give some warning before we do that. The imports have always been functions, so making the exports functions as well offers better consistency. I am sure there are other benefits too.

If you are new to build123d, I highly recommend you go through the examples here which covers some common use cases -- including this one. https://build123d.readthedocs.io/en/latest/introductory_examples.html

@DeflateAwning
Copy link
Author

In most other similar DSL-like systems (e.g., Polars, Pandas, etc.), the imports are viewed as a constructor-like entry point, and thus are not tied to an object. However, exports are an operation on an object (exporting that object), and thus are a method of that object.

I've done that introductory example. Quite good overall.

I just think this is a very strange choice, and that the reasoning isn't quite there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants