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

Feat/issue 504 nearest #540

Merged
merged 2 commits into from
Jul 28, 2021
Merged

Feat/issue 504 nearest #540

merged 2 commits into from
Jul 28, 2021

Conversation

jgd10
Copy link
Contributor

@jgd10 jgd10 commented Jul 20, 2021

NNEAR
KNEAR
ENEARN

  • tests

@jgd10 jgd10 requested a review from akaszynski July 20, 2021 11:38
This was referenced Jul 20, 2021
>>> mapdl.block(0, 10, 0, 10, 0, 10)
>>> mapdl.esize(3)
>>> mapdl.vmesh('ALL')
>>> q = mapdl.query()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought about this more and I think this should be an attribute. That way it matches the rest of our API and we can do this:

node_number = mapdl.query.node(5, 5, 5)

You'll also be able be able use autocomplete/intellisense right on mapdl.query, making it easy to inspect the attr for available methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've also been thinking about this and I've realised what I don't like about query as a property. It's more to do with the word itself. Query is a verb and a noun, so I find I expect one usage and see another, which I really don't like. I think some other name would be more appropriate, that is less ambiguous. E.g.

  • querier or queries
  • functions - since we are providing access to the APDL functions after all, I think this would make most sense and be the most intuitive
  • query_service - most accurate, but longest

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm out of the office, but this has been eating away from me.

I feel like queries fits best here. We're effectively providing a collection of methods to allow the user easier access to MAPDL *GET methods.

node_number = mapdl.queries.node(5, 5, 5)  # BTW, IMO should be "node_num_from_coord"

functions isn't the best fit since we're already providing access to MAPDL functions like K, VPLOT, VGET, etc. Queries makes a bit more sense since we're querying the database for certain values.

Let's merge this PR and create a new one refactoring this if you're fine with queries. Otherwise, we can continue the discussion here or on teams.

@akaszynski akaszynski merged commit f190bcc into main Jul 28, 2021
@germa89 germa89 deleted the feat/issue_504_nearest branch May 31, 2022 09:37
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.

2 participants