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.

Sign up for GitHub

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

Jump to bottom

Simplify function names in Data classes #201

Closed
aiida-bot opened this issue May 11, 2016 · 6 comments
Closed

Simplify function names in Data classes #201

aiida-bot opened this issue May 11, 2016 · 6 comments
Assignees
@astamminger @elsapassaro
Labels
priority/nice-to-have topic/naming-issues type/task
Milestone
v1.0.0b1

Comments

@aiida-bot
Copy link

Originally reported by: Giovanni Pizzi (Bitbucket: pizzi, GitHub: giovannipizzi)

Some methods are not called intuitively:

  • ArrayData.arraynames() is actually a method and not a property, so should rather be .get_arraynames()
  • TrajectoryData: step and stepid are confused. Proposed changes include step_to_structure -> get_step_structure; get_steps -> get_stepids; get_step_index -> get_index_from_stepid (in the DB we will still call it 'step')

Also, we should redesign the node class (and its subclasses) to avoid that there are too many methods 'cluttering' the namespace for every node (to discuss)
@aiida-bot
Copy link
Author

Original comment by Giovanni Pizzi (Bitbucket: pizzi, GitHub: giovannipizzi):

Mentioned in c198bdc

@aiida-bot
Copy link
Author

Original comment by Nicolas Mounet (Bitbucket: mounet, GitHub: nmounet):

Maybe also:

CifData._get_aiida_structure -> CifData.get_structure

@aiida-bot
Copy link
Author

Original comment by Leonid Kahle (Bitbucket: leonidkahle, GitHub: lekah):

TrajectoryData.set_trajectory(): make setting of steps (or stepids) and cells optional, store symbols in the attributes (easier querying)

@giovannipizzi giovannipizzi added this to the 1.0 release milestone Feb 28, 2017
@DropD
Copy link
Contributor

DropD commented Dec 18, 2017 •
edited
Loading

I will keep this issue for the renaming suggestions and split out the general Node design discussion into a new issue (#1004).

Summary of methods to be renamed:

  • ArrayData:
    • .arraynames -> .get_arraynames
  • TrajectoryData:
    • .step_to_structure -> .get_step_structure
    • .get_steps -> .get_stepids
    • .get_step_index -> .get_index_from_stepid
  • CifData
    • ._get_aiida_structure -> .get_structure

Summary of proposed other changes:

  • TrajectoryData:
    • .set_trajectory(): make setting of steps (or stepids) and cells optional, store symbols in the attributes (easier querying)

@DropD DropD mentioned this issue Dec 18, 2017
Closed
@DropD DropD changed the title Simplify function names and reduce methods in the Node namespace Simplify function names in Data classes Dec 18, 2017
@sphuber sphuber modified the milestones: 1.0 release, v1.0.0 May 9, 2018
@sphuber sphuber modified the milestones: v1.0.0, v1.0.0b1 Dec 4, 2018
@astamminger astamminger assigned astamminger and unassigned lekah and giovannipizzi Dec 5, 2018
astamminger added a commit to boschresearch/aiida_core that referenced this issue Dec 6, 2018
@astamminger
Store symbols in TrajectoryData attribute instead of array
8f2c928 
As proposed in issue aiidateam#201 symbols are now stored in a
TrajectoryData attribute rather than an array to simplify the
query for these symbols
@astamminger astamminger mentioned this issue Dec 6, 2018
Merged
giovannipizzi pushed a commit that referenced this issue Jan 14, 2019
@astamminger @giovannipizzi
Introduce more intuitive method names in Data classes and reduce requ…
8398f24 
…ired input parameter for TrajectoryData.set_trajectory() method (#2310)

* Rename ArrayData.iterarrays() to ArrayData.get_iterarrays()

* [TrajectoryData] Rename _get_aiida_structure() -> get_structure()

* [TrajectoryData] Rename _get_cif() -> get_cif()

* Rename _get_cif() -> get_cit() in tcod.py

* [StructureData] Rename _get_cif() -> get_cif()

* Make stepid and cells parameter optional
  Change TrajectoryData to make passing stepids and cells optional. While
  nothing will be stored for cells if not given a consecutive sequence will be
  automatically assigned to stepids if missing from the inputs.

* Store symbols in TrajectoryData attribute instead of array
  As proposed in issue #201 symbols are now stored in a
  TrajectoryData attribute rather than an array to simplify the
  query for these symbols

* Change passed symbols from numpy.ndarray to list in set_structurelist

* Change TrajectoryData tests to use symbols attribute rather than symbols array

* Make Code.is_hidden() available as class property

* Rename Code.full_text_info() method to .get_full_text_info()

* Change RemoteData.is_empty() method to class property .is_empty

* Change is_alloy() and has_vacancies() methods to properties

  Make class methods .is_alloy() and .has_vacancies() for StrutureData
  and Kind classes accessible as class properties .is_alloy and
  .has_vacancies

* Change symbols type from numpy.array to list

* Add deprecation warning to renamed methods

* Rename code attribute is_hidden to hidden.

* Set allowed symbols type to be Iterable rather than list

* Ensure symbols are stored as list

* Adjust argument ordering in set_trajectory() docstring.

* Document introduced changes in concepts/workflows
@elsapassaro
Copy link
Contributor

Added migration and test for django, pushed to https://github.com/asle85/aiida_core/tree/fix_201_method_names_for_data

To do: write migration for sqlalchemy

@elsapassaro elsapassaro mentioned this issue Feb 8, 2019
Merged
@sphuber
Copy link
Contributor

sphuber commented Feb 8, 2019

Fixed in PR #2422

@sphuber sphuber closed this as completed Feb 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@astamminger astamminger

@elsapassaro elsapassaro

Labels
priority/nice-to-have topic/naming-issues type/task
Projects
None yet
Milestone
v1.0.0b1
Development

No branches or pull requests
7 participants
@DropD @sphuber @astamminger @elsapassaro @lekah @giovannipizzi @aiida-bot