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

Update parameter names in FEModel3D methods for consistency of API #194

Merged
merged 1 commit into from
May 6, 2024

Conversation

connorferster
Copy link
Contributor

Hi @JWock82,

I noticed that there were some breaking changes to the API with recent versions but that the updates were not applied to all of the methods in FEModel3D. I have updated the rest of the parameter names across the FEModel3D object by following what seemed to be the desired new convention. Here is how I understand it:

  • When creating a new object (e.g. node, member, section, etc.), the method that creates it (e.g. .add_node) will accept a parameter called name to set the name of the object in the model.
  • When creating an assignment to an existing object (e.g. a load to a member), then the parameter will be named something like member_name (as opposed to Member) to make it clear that this parameter is to accept a name and not a Member object itself.

I ran the test suite and all tests pass. However, I notice that the test suite does not use any keyword arguments when the methods are called. The tests passing will reflect that the parameter name changes I made in the function signatures have been updated in the function implementations and not that I have followed the naming convention perfectly (although, from manual review, I think I have).

Let me know your thoughts on this?

@connorferster
Copy link
Contributor Author

Here is an example (from the docs) of what I meant when I referred to changes to the API not being propagated across the FEModel3D methods:

image

@connorferster
Copy link
Contributor Author

connorferster commented May 5, 2024

I also change the Direction parameter to direction but left capitalization in certain parameter names where "FEA conventions" make more sense (e.g. X, Y, Z for global coordinates; E and I instead of e and i (because those are mathematical constants).

@JWock82 JWock82 merged commit 77db265 into JWock82:main May 6, 2024
4 checks passed
@JWock82
Copy link
Owner

JWock82 commented May 6, 2024

Thanks Connor. I merged your pull request. I've tried to merge my coding style with the PEP8 style guide as much as makes sense for FEA. Some of this is stuff I coded early on when I was still learning PEP8. I appreciate your help. The docs should be updated too.

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