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

MEPDesign Inclusion #25

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

andrerdsilva
Copy link

@andrerdsilva andrerdsilva commented Feb 21, 2021

MEPDesign to pyRevitMEP. In soon in my youtube channel (Engenheiro Hidraúlico) I'll explain how works the code panel and much more.
Thanks!

MEPDesign to pyRevitMEP
@CyrilWaechter
Copy link
Owner

CyrilWaechter commented Feb 28, 2021

Hi André :-),

Thanks for your pull request.

General

  • variable names are not PEP8 compliant. You can use a linter like pylint to help you.
  • I understand the wish of using variable names in your own language (my native language is french ;-)) but you should consider using English variable names and comments to allow more people to understand your code without effort.
  • Currently it whole panel will not fit on a laptop. It would be great to add more dropdown button or stack to reduce panel size. (pyRevit and pyRevitMEP before this pull request have about the same size for this reason)

Tools

Split pipes

  • No unit is displayed for distance. I see in code that you statically convert input meters into internal units. You could use UnitUtils instead to allow user to input in project units then use ConvertToInternalUnits method to automatically convert it.
  • Suggestion : Use alternate click to configure the tool and simple click to process without having to configure each time.
  • Many loops could be simplified using for i, value in enumerate(values) instead of for i in range(values) then assign by index

Code - PipeConnection

Can you explain the goal of this function ? I was not able to make it work. It require special parameters ?

Renumber mark

Same

Tell me if you need more information on some points. And again, congratulation for doing this already and thanks for your work.

@andrerdsilva
Copy link
Author

Hello Cyril :-),

I will correct the problems, soon I will send a new pull request.

Code - PipeConnection

This function searches for a .csv file containing the names of the families and also the parameters that must be evaluated to assign a code and description for the families. It is possible in the same .csv file to configure the codes and descriptions used by several construction companies, without inserting this information through lookuptable in families.

Renumber mark

With the descriptions inserted in the project with the previous function, we can number the MARK parameter considering the description of the elements in alphabetical order.

I am putting this link of a video in which I demonstrate the use of the CODES and MARK PARAMETER tab, I intend to remove these functions, but if you want to test these functions I am attaching the .csv file with the registered families here, I registered all the families of Amanco Wavin for Brazil.

Codes.zip

The functions of the CODE and RENUMBER MARK tab are very specific, I think they are not so interesting for the community, in the next pull request I intend to remove them.

Thanks.

@CyrilWaechter
Copy link
Owner

Hi André,

I watched your video. Now I understand. I think it is a good idea to keep it but maybe put it in a stack with dropdowns.

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

Successfully merging this pull request may close these issues.

None yet

2 participants