Skip to content

83 clear up classes in sample#87

Merged
andped10 merged 20 commits into
developfrom
83-clear-up-classes-in-sample
Feb 15, 2024
Merged

83 clear up classes in sample#87
andped10 merged 20 commits into
developfrom
83-clear-up-classes-in-sample

Conversation

@andped10
Copy link
Copy Markdown
Collaborator

@andped10 andped10 commented Feb 9, 2024

This PR only consists only code refactoring and typing.

Entry to all sample classes should be fetched directly from sample

This is obtained by the __init__.py file in the sample folder. By doing so we will be free to make changes to the internal structure of the module in the future.

The internal structure of the sample folder is now:

sample

  • elementals
    • layers (layer consists of a material)
      • layer
      • layer_apm
    • materials (most fundamental element)
      • material
      • material_density
      • material_mixture
  • assemblies (these are constructs of layers)
    • gradient_layer
    • multilayer
    • repeating_multilayer
    • surfactant_layer

However from a user perspective you would get the Multilayer class implemented in sample.assemblies.multilayer by executing:
from sample import Multiplayer

or for Layer implemented in sample.elementals.layers.layer:
from sample import Layer

Other Changes

The modules for material and layer been split into several files only containing a single class per file.

Class renaming:

  • Structure has been renamed to Sample
  • Layers has been renamed to LayerCollection
  • Materials has been renamed to MaterialCollection

Base classes introduced in elementals:

  • BaseElement
  • BaseElementCollection

ADR is created in #89

@andped10 andped10 marked this pull request as ready for review February 9, 2024 14:03
rozyczko

This comment was marked as resolved.

Copy link
Copy Markdown
Member

@rozyczko rozyczko left a comment

Choose a reason for hiding this comment

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

Small comment on naming consistency.

module surfactant_layer contains class SurfactantLayer.
module gradient_layer contains class GradientLayer

so it is sensible to assume that PascalCase for classes converts into snake_case for modules.

This doesn't seem to hold for MultiLayer class which module you named multilayer.
Is there any reason or should the class name be just Multilayer (lower case l)?

Copy link
Copy Markdown
Member

@rozyczko rozyczko left a comment

Choose a reason for hiding this comment

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

A couple of base.py modules should also be renamed according to the class name inside.
I.e.
sample/assemblies/base.py -> base_assembly.py
sample/elementals/base.py -> base_element.py

Copy link
Copy Markdown
Member

@rozyczko rozyczko left a comment

Choose a reason for hiding this comment

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

The base class names in sample/elementals/base.py are BaseElement and BaseElementCollection.
Should the folder name be elements then?

@andped10
Copy link
Copy Markdown
Collaborator Author

The base class names in sample/elementals/base.py are BaseElement and BaseElementCollection. Should the folder name be elements then?

Changed folder name to elements

@andped10
Copy link
Copy Markdown
Collaborator Author

Small comment on naming consistency.

module surfactant_layer contains class SurfactantLayer. module gradient_layer contains class GradientLayer

so it is sensible to assume that PascalCase for classes converts into snake_case for modules.

This doesn't seem to hold for MultiLayer class which module you named multilayer. Is there any reason or should the class name be just Multilayer (lower case l)?

Changed class names containing MultiLayer to Multilayer

@andped10
Copy link
Copy Markdown
Collaborator Author

A couple of base.py modules should also be renamed according to the class name inside. I.e. sample/assemblies/base.py -> base_assembly.py sample/elementals/base.py -> base_element.py

Renamed files and extracted base for base_element_collection.py

@andped10 andped10 requested a review from rozyczko February 13, 2024 10:52
Copy link
Copy Markdown
Member

@rozyczko rozyczko left a comment

Choose a reason for hiding this comment

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

I think this is now ready for mergig.

@andped10 andped10 merged commit 34db684 into develop Feb 15, 2024
@andped10 andped10 deleted the 83-clear-up-classes-in-sample branch February 15, 2024 05:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants